Skip to content

Commit e00d4b5

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 a9ab0cf commit e00d4b5

File tree

3 files changed

+19
-16
lines changed

3 files changed

+19
-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(() => 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

+5-12
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe("Heart", () => {
2323
})
2424
afterEach(() => {
2525
jest.resetAllMocks()
26+
jest.useRealTimers()
2627
if (heart) {
2728
heart.dispose()
2829
}
@@ -42,11 +43,7 @@ describe("Heart", () => {
4243
expect(fileContents).toBe(text)
4344

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

6962
const isAlive = heart.alive()
7063
expect(isAlive).toBe(true)

0 commit comments

Comments
 (0)