Write tests for heartbeats

Test the expected behaviour of the heartbeat module against mocked
Nock requests.

Return the same promise from `Appsignal.heartbeat()` that was
returned by the function given to it as an argument, instead of
returning a wrapper promise that emits the heartbeat.

Fix a bug where the timestamp was sent in milliseconds instead of
seconds.

Author: unflxw

.changesets/add-heartbeats-support.md

@@ -34,9 +34,8 @@ reported to AppSignal, triggering a notification about the missing heartbeat.
 The exception will bubble outside of the heartbeat function.
 
 If the function passed to `heartbeat` returns a promise, the finish event will
-be reported to AppSignal if the promise resolves, and a wrapped promise will
-be returned, which can be awaited. This means that you can use heartbeats to
-track the duration of async functions:
+be reported to AppSignal if the promise resolves. This means that you can use
+heartbeats to track the duration of async functions:
 
 ```javascript
 import { heartbeat } from "@appsignal/nodejs"

src/__tests__/heartbeat.test.ts

@@ -0,0 +1,274 @@
+import nock, { Scope } from "nock"
+import { heartbeat, Heartbeat, EventKind } from "../heartbeat"
+import { Client, Options } from "../client"
+
+const DEFAULT_CLIENT_CONFIG: Partial<Options> = {
+  active: true,
+  name: "Test App",
+  pushApiKey: "test-push-api-key",
+  environment: "test",
+  hostname: "test-hostname"
+}
+
+function mockHeartbeatRequest(
+  kind: EventKind,
+  { delay } = { delay: 0 }
+): Scope {
+  return nock("https://appsignal-endpoint.net:443")
+    .post("/heartbeats/json", body => {
+      return body.name === "test-heartbeat" && body.kind === kind
+    })
+    .query({
+      api_key: "test-push-api-key",
+      name: "Test App",
+      environment: "test",
+      hostname: "test-hostname"
+    })
+    .delay(delay)
+    .reply(200, "")
+}
+
+function nextTick(fn: () => void): Promise<void> {
+  return new Promise(resolve => {
+    process.nextTick(() => {
+      fn()
+      resolve()
+    })
+  })
+}
+
+function sleep(ms: number): Promise<void> {
+  return new Promise(resolve => {
+    setTimeout(resolve, ms)
+  })
+}
+
+function interceptRequestBody(scope: Scope): Promise<string> {
+  return new Promise(resolve => {
+    scope.on("request", (_req, _interceptor, body: string) => {
+      resolve(body)
+    })
+  })
+}
+
+describe("Heartbeat", () => {
+  let client: Client
+  let theHeartbeat: Heartbeat
+
+  beforeAll(() => {
+    theHeartbeat = new Heartbeat("test-heartbeat")
+
+    if (!nock.isActive()) {
+      nock.activate()
+    }
+  })
+
+  beforeEach(() => {
+    client = new Client(DEFAULT_CLIENT_CONFIG)
+
+    nock.cleanAll()
+    nock.disableNetConnect()
+  })
+
+  afterEach(() => {
+    client.stop()
+  })
+
+  afterAll(() => {
+    nock.restore()
+  })
+
+  it("does not transmit any events when AppSignal is not active", async () => {
+    client.stop()
+    client = new Client({
+      ...DEFAULT_CLIENT_CONFIG,
+      active: false
+    })
+
+    const startScope = mockHeartbeatRequest("start")
+    const finishScope = mockHeartbeatRequest("finish")
+
+    await expect(theHeartbeat.start()).resolves.toBeUndefined()
+    await expect(theHeartbeat.finish()).resolves.toBeUndefined()
+
+    expect(startScope.isDone()).toBe(false)
+    expect(finishScope.isDone()).toBe(false)
+  })
+
+  it("heartbeat.start() sends a heartbeat start event", async () => {
+    const scope = mockHeartbeatRequest("start")
+
+    await expect(theHeartbeat.start()).resolves.toBeUndefined()
+
+    scope.done()
+  })
+
+  it("heartbeat.finish() sends a heartbeat finish event", async () => {
+    const scope = mockHeartbeatRequest("finish")
+
+    await expect(theHeartbeat.finish()).resolves.toBeUndefined()
+
+    scope.done()
+  })
+
+  it("Heartbeat.shutdown() awaits pending heartbeat event promises", async () => {
+    const startScope = mockHeartbeatRequest("start", { delay: 100 })
+    const finishScope = mockHeartbeatRequest("finish", { delay: 200 })
+
+    let finishPromiseResolved = false
+    let shutdownPromiseResolved = false
+
+    const startPromise = theHeartbeat.start()
+
+    theHeartbeat.finish().then(() => {
+      finishPromiseResolved = true
+    })
+
+    const shutdownPromise = Heartbeat.shutdown().then(() => {
+      shutdownPromiseResolved = true
+    })
+
+    await expect(startPromise).resolves.toBeUndefined()
+
+    // The finish promise should still be pending, so the shutdown promise
+    // should not be resolved yet.
+    await nextTick(() => {
+      expect(finishPromiseResolved).toBe(false)
+      expect(shutdownPromiseResolved).toBe(false)
+    })
+
+    startScope.done()
+
+    // The shutdown promise should not resolve until the finish promise
+    // resolves.
+    await expect(shutdownPromise).resolves.toBeUndefined()
+
+    await nextTick(() => {
+      expect(finishPromiseResolved).toBe(true)
+    })
+
+    finishScope.done()
+  })
+
+  describe("Appsignal.heartbeat()", () => {
+    it("without a function, sends a heartbeat finish event", async () => {
+      const startScope = mockHeartbeatRequest("start")
+      const finishScope = mockHeartbeatRequest("finish")
+
+      expect(heartbeat("test-heartbeat")).toBeUndefined()
+
+      await nextTick(() => {
+        expect(startScope.isDone()).toBe(false)
+        finishScope.done()
+      })
+    })
+
+    describe("with a function", () => {
+      it("sends heartbeat start and finish events", async () => {
+        const startScope = mockHeartbeatRequest("start")
+        const startBody = interceptRequestBody(startScope)
+
+        const finishScope = mockHeartbeatRequest("finish")
+        const finishBody = interceptRequestBody(finishScope)
+
+        expect(
+          heartbeat("test-heartbeat", () => {
+            const thisSecond = Math.floor(Date.now() / 1000)
+
+            // Since this function must be synchronous, we need to deadlock
+            // until the next second in order to obtain different timestamps
+            // for the start and finish events.
+            // eslint-disable-next-line no-constant-condition
+            while (true) {
+              if (Math.floor(Date.now() / 1000) != thisSecond) break
+            }
+
+            return "output"
+          })
+        ).toBe("output")
+
+        // Since the function is synchronous and deadlocks, the start and
+        // finish events' requests are actually initiated simultaneously
+        // afterwards, when the function finishes and the event loop ticks.
+        await nextTick(() => {
+          startScope.done()
+          finishScope.done()
+        })
+
+        expect(JSON.parse(await finishBody).timestamp).toBeGreaterThan(
+          JSON.parse(await startBody).timestamp
+        )
+      })
+
+      it("does not send a finish event when the function throws an error", async () => {
+        const startScope = mockHeartbeatRequest("start")
+        const finishScope = mockHeartbeatRequest("finish")
+
+        expect(() => {
+          heartbeat("test-heartbeat", () => {
+            throw new Error("thrown")
+          })
+        }).toThrow("thrown")
+
+        await nextTick(() => {
+          startScope.done()
+          expect(finishScope.isDone()).toBe(false)
+        })
+      })
+    })
+
+    describe("with an async function", () => {
+      it("sends heartbeat start and finish events", async () => {
+        const startScope = mockHeartbeatRequest("start")
+        const startBody = interceptRequestBody(startScope)
+
+        const finishScope = mockHeartbeatRequest("finish")
+        const finishBody = interceptRequestBody(finishScope)
+
+        await expect(
+          heartbeat("test-heartbeat", async () => {
+            await nextTick(() => {
+              startScope.done()
+              expect(finishScope.isDone()).toBe(false)
+            })
+
+            const millisecondsToNextSecond = 1000 - (Date.now() % 1000)
+            await sleep(millisecondsToNextSecond)
+
+            return "output"
+          })
+        ).resolves.toBe("output")
+
+        await nextTick(() => {
+          startScope.done()
+          finishScope.done()
+        })
+
+        expect(JSON.parse(await finishBody).timestamp).toBeGreaterThan(
+          JSON.parse(await startBody).timestamp
+        )
+      })
+
+      it("does not send a finish event when the promise returned is rejected", async () => {
+        const startScope = mockHeartbeatRequest("start")
+        const finishScope = mockHeartbeatRequest("finish")
+
+        await expect(
+          heartbeat("test-heartbeat", async () => {
+            await nextTick(() => {
+              startScope.done()
+              expect(finishScope.isDone()).toBe(false)
+            })
+
+            throw new Error("rejected")
+          })
+        ).rejects.toThrow("rejected")
+
+        await nextTick(() => {
+          startScope.done()
+          expect(finishScope.isDone()).toBe(false)
+        })
+      })
+    })
+  })
+})

src/__tests__/transmitter.test.ts

@@ -33,7 +33,10 @@ describe("Transmitter", () => {
       return new Transmitter("https://example.com/foo", "request body")
     }
 
-    function mockSampleRequest(responseBody: object | string, query = {}): Scope {
+    function mockSampleRequest(
+      responseBody: object | string,
+      query = {}
+    ): Scope {
       return nock("https://example.com")
         .post("/foo", "request body")
         .query({
@@ -62,7 +65,7 @@ describe("Transmitter", () => {
       const scope = mockSampleRequest({ json: "response" })
 
       await expectResponse(transmitter.transmit(), { json: "response" })
-      
+
       scope.done()
     })
 
@@ -115,7 +118,9 @@ describe("Transmitter", () => {
     const transmitter = new Transmitter("http://example.com/foo")
 
     it("resolves to a response stream on success", async () => {
-      const scope = nock("http://example.com").get("/foo").reply(200, "response body")
+      const scope = nock("http://example.com")
+        .get("/foo")
+        .reply(200, "response body")
 
       const stream = await transmitter.downloadStream()
 
@@ -129,7 +134,9 @@ describe("Transmitter", () => {
     })
 
     it("rejects if the status code is not successful", async () => {
-      const scope = nock("http://example.com").get("/foo").reply(404, "not found")
+      const scope = nock("http://example.com")
+        .get("/foo")
+        .reply(404, "not found")
 
       await expect(transmitter.downloadStream()).rejects.toMatchObject({
         kind: "statusCode",
@@ -204,7 +211,9 @@ describe("Transmitter", () => {
     }
 
     it("performs an HTTP GET request", async () => {
-      const scope = nock("http://example.invalid").get("/foo").reply(200, "response body")
+      const scope = nock("http://example.invalid")
+        .get("/foo")
+        .reply(200, "response body")
 
       const { callback, onData, onError } = await transmitterRequest(
         "GET",

src/heartbeat.ts

@@ -51,7 +51,7 @@ export class Heartbeat {
       name: this.name,
       id: this.id,
       kind: kind,
-      timestamp: Date.now()
+      timestamp: Math.floor(Date.now() / 1000)
     }
   }
 
@@ -106,10 +106,7 @@ export function heartbeat<T>(name: string, fn?: () => T): T | undefined {
   }
 
   if (output instanceof Promise) {
-    output = output.then(result => {
-      heartbeat.finish()
-      return result
-    }) as typeof output
+    output.then(() => heartbeat.finish()).catch(() => {})
   } else {
     heartbeat.finish()
   }