Skip to content

Commit 838db7d

Browse files
committed
refactor: make Heart.beat() async
This allows us to properly await heart.beat() in our tests and remove the HACK I added before.
1 parent 8e7ccbf commit 838db7d

File tree

3 files changed

+18
-16
lines changed

3 files changed

+18
-16
lines changed

src/node/heart.ts

+12-4
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,28 @@ export class Heart {
2020
* timeout and start or reset a timer that keeps running as long as there is
2121
* activity. Failures are logged as warnings.
2222
*/
23-
public beat(): void {
23+
public async beat(): Promise<void> {
2424
if (this.alive()) {
2525
return
2626
}
2727

2828
logger.trace("heartbeat")
29-
fs.writeFile(this.heartbeatPath, "").catch((error) => {
29+
try {
30+
await fs.writeFile(this.heartbeatPath, "")
31+
} catch (error: any) {
3032
logger.warn(error.message)
31-
})
33+
}
3234
this.lastHeartbeat = Date.now()
3335
if (typeof this.heartbeatTimer !== "undefined") {
3436
clearTimeout(this.heartbeatTimer)
3537
}
36-
this.heartbeatTimer = setTimeout(async () => await heartbeatTimer(this.isActive, this.beat), this.heartbeatInterval)
38+
this.heartbeatTimer = setTimeout(async () => {
39+
try {
40+
await heartbeatTimer(this.isActive, this.beat)
41+
} catch (error: any) {
42+
logger.warn(error.message)
43+
}
44+
}, this.heartbeatInterval)
3745
}
3846

3947
/**

src/node/routes/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ export const register = async (app: App, args: DefaultedArgs): Promise<Disposabl
5656
// /healthz|/healthz/ needs to be excluded otherwise health checks will make
5757
// it look like code-server is always in use.
5858
if (!/^\/healthz\/?$/.test(req.url)) {
59+
// NOTE@jsjoeio - intentionally not awaiting the .beat() call here because
60+
// we don't want to slow down the request.
5961
heart.beat()
6062
}
6163

test/unit/node/heart.test.ts

+4-12
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,7 @@ describe("Heart", () => {
4343
expect(fileContents).toBe(text)
4444

4545
heart = new Heart(pathToFile, mockIsActive(true))
46-
heart.beat()
47-
// HACK@jsjoeio - beat has some async logic but is not an async method
48-
// Therefore, we have to create an artificial wait in order to make sure
49-
// all async code has completed before asserting
50-
await new Promise((r) => setTimeout(r, 100))
46+
await heart.beat()
5147
// Check that the heart wrote to the heartbeatFilePath and overwrote our text
5248
const fileContentsAfterBeat = await readFile(pathToFile, { encoding: "utf8" })
5349
expect(fileContentsAfterBeat).not.toBe(text)
@@ -57,15 +53,11 @@ describe("Heart", () => {
5753
})
5854
it("should log a warning when given an invalid file path", async () => {
5955
heart = new Heart(`fakeDir/fake.txt`, mockIsActive(false))
60-
heart.beat()
61-
// HACK@jsjoeio - beat has some async logic but is not an async method
62-
// Therefore, we have to create an artificial wait in order to make sure
63-
// all async code has completed before asserting
64-
await new Promise((r) => setTimeout(r, 100))
56+
await heart.beat()
6557
expect(logger.warn).toHaveBeenCalled()
6658
})
67-
it("should be active after calling beat", () => {
68-
heart.beat()
59+
it("should be active after calling beat", async () => {
60+
await heart.beat()
6961

7062
const isAlive = heart.alive()
7163
expect(isAlive).toBe(true)

0 commit comments

Comments
 (0)