Archived
1
0

refactor(heart): bind class methods and make beat async (#5142)

* feat: set up new test for beat twice

* refactor: make Heart.beat() async

This allows us to properly await heart.beat() in our tests and remove
the HACK I added before.

* refactor: bind heart methods .beat and .alive

This allows the functions to maintain access to the Heart instance (or
`this`) even when they are passed to other functions. We do this because
we pass both `isActive` and `beat` to `heartbeatTimer`.

* feat(heart): add test to ensure no warnings called

* fixup!: revert setTimeout for heartbeatTimer

* fixup!: return promise in beat
This commit is contained in:
Joe Previte 2022-05-04 16:05:48 -07:00 committed by GitHub
parent 7027ec7d60
commit 88e971c609
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 28 additions and 17 deletions

View File

@ -9,7 +9,10 @@ export class Heart {
private heartbeatInterval = 60000
public lastHeartbeat = 0
public constructor(private readonly heartbeatPath: string, private readonly isActive: () => Promise<boolean>) {}
public constructor(private readonly heartbeatPath: string, private readonly isActive: () => Promise<boolean>) {
this.beat = this.beat.bind(this)
this.alive = this.alive.bind(this)
}
public alive(): boolean {
const now = Date.now()
@ -20,20 +23,22 @@ export class Heart {
* timeout and start or reset a timer that keeps running as long as there is
* activity. Failures are logged as warnings.
*/
public beat(): void {
public async beat(): Promise<void> {
if (this.alive()) {
return
}
logger.trace("heartbeat")
fs.writeFile(this.heartbeatPath, "").catch((error) => {
logger.warn(error.message)
})
this.lastHeartbeat = Date.now()
if (typeof this.heartbeatTimer !== "undefined") {
clearTimeout(this.heartbeatTimer)
}
this.heartbeatTimer = setTimeout(() => heartbeatTimer(this.isActive, this.beat), this.heartbeatInterval)
try {
return await fs.writeFile(this.heartbeatPath, "")
} catch (error: any) {
logger.warn(error.message)
}
}
/**

View File

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

View File

@ -23,6 +23,7 @@ describe("Heart", () => {
})
afterEach(() => {
jest.resetAllMocks()
jest.useRealTimers()
if (heart) {
heart.dispose()
}
@ -42,11 +43,7 @@ describe("Heart", () => {
expect(fileContents).toBe(text)
heart = new Heart(pathToFile, mockIsActive(true))
heart.beat()
// HACK@jsjoeio - beat has some async logic but is not an async method
// Therefore, we have to create an artificial wait in order to make sure
// all async code has completed before asserting
await new Promise((r) => setTimeout(r, 100))
await heart.beat()
// Check that the heart wrote to the heartbeatFilePath and overwrote our text
const fileContentsAfterBeat = await readFile(pathToFile, { encoding: "utf8" })
expect(fileContentsAfterBeat).not.toBe(text)
@ -56,15 +53,11 @@ describe("Heart", () => {
})
it("should log a warning when given an invalid file path", async () => {
heart = new Heart(`fakeDir/fake.txt`, mockIsActive(false))
heart.beat()
// HACK@jsjoeio - beat has some async logic but is not an async method
// Therefore, we have to create an artificial wait in order to make sure
// all async code has completed before asserting
await new Promise((r) => setTimeout(r, 100))
await heart.beat()
expect(logger.warn).toHaveBeenCalled()
})
it("should be active after calling beat", () => {
heart.beat()
it("should be active after calling beat", async () => {
await heart.beat()
const isAlive = heart.alive()
expect(isAlive).toBe(true)
@ -75,6 +68,17 @@ describe("Heart", () => {
const isAlive = heart.alive()
expect(isAlive).toBe(false)
})
it("should beat twice without warnings", async () => {
// Use fake timers so we can speed up setTimeout
jest.useFakeTimers()
heart = new Heart(`${testDir}/hello.txt`, mockIsActive(true))
await heart.beat()
// we need to speed up clocks, timeouts
// call heartbeat again (and it won't be alive I think)
// then assert no warnings were called
jest.runAllTimers()
expect(logger.warn).not.toHaveBeenCalled()
})
})
describe("heartbeatTimer", () => {