Skip to content

Commit

Permalink
fix(process): rename heartbeat to ping and clear timeout on response
Browse files Browse the repository at this point in the history
This commit addresses two issues in the worker communication system. First, it renames the 'heartbeat' message to 'ping' for clarity and consistency across the codebase. This change affects the `workerRegister.ts`, `workerRegister.test.ts`, and `workerRequest.ts` files.

Secondly, it fixes a potential issue with dangling timeouts in the `workerRequest` function. The timeout is now cleared not only when receiving a ping but also after resolving or rejecting the promise. This ensures that the timeout is properly cleaned up in all scenarios, preventing potential memory leaks or unexpected behavior.
  • Loading branch information
shorwood committed Dec 29, 2024
1 parent cd75480 commit 23b1f7c
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 9 deletions.
12 changes: 6 additions & 6 deletions packages/process/workerRegister.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ describe('workerRegister', () => {
expect(callback).toHaveBeenCalledWith(1, 2)
})

it('should send a heartbeat message to the worker', async() => {
it('should send a ping message to the worker', async() => {
const { port1, port2 } = new MessageChannel()
workerRegister('fn', () => {})
parentPort!.postMessage({ name: 'fn', parameters: [], port: port1 })
const message = await new Promise<MessageEvent>(resolve => port2.addEventListener('message', resolve))
expect(message.data).toBe('heartbeat')
expect(message.data).toBe('ping')
})

it('should send the result of the function back to the worker', async() => {
Expand All @@ -80,7 +80,7 @@ describe('workerRegister', () => {
parentPort!.postMessage({ name: 'fn', parameters: ['world'], port: port1 })
const message = await new Promise<MessageEvent>((resolve) => {
port2.addEventListener('message', (data) => {
if (data.data !== 'heartbeat') resolve(data)
if (data.data !== 'ping') resolve(data)
})
})
expect(message.data).toStrictEqual({
Expand All @@ -94,7 +94,7 @@ describe('workerRegister', () => {
parentPort!.postMessage({ name: 'fn', parameters: [], port: port1 })
const message = await new Promise<MessageEvent>((resolve) => {
port2.addEventListener('message', (data) => {
if (data.data !== 'heartbeat') resolve(data)
if (data.data !== 'ping') resolve(data)
})
})
expect(message.data).toStrictEqual({
Expand All @@ -107,7 +107,7 @@ describe('workerRegister', () => {
parentPort!.postMessage({ name: 'fn', parameters: [], port: port1 })
const message = await new Promise<MessageEvent>((resolve) => {
port2.addEventListener('message', (data) => {
if (data.data !== 'heartbeat') resolve(data)
if (data.data !== 'ping') resolve(data)
})
})
expect(message.data).toStrictEqual({
Expand All @@ -125,7 +125,7 @@ describe('workerRegister', () => {
return new Promise((resolve, reject) => {
port2.addEventListener('error', reject)
port2.addEventListener('message', (response) => {
if (response.data === 'heartbeat') return
if (response.data === 'ping') return
resolve(response.data as WorkerResponse<number>)
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/process/workerRegister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ async function requestCallback(request: WorkerRequest): Promise<void> {

// --- If the handler is registered, call it.
try {
port.postMessage('heartbeat')
port.postMessage('ping')
const value = await fn!(...parameters) as unknown
port.postMessage({ value })
}
Expand Down
5 changes: 3 additions & 2 deletions packages/process/workerRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,12 @@ export async function workerRequest<T extends Function>(worker: Worker, options:
const timeoutInstnace = setTimeout(reject, timeout, error)
port2.once('error', reject)
port2.once('messageerror', reject)
port2.addListener('message', (response: 'heartbeat' | WorkerResponse) => {
if (response === 'heartbeat') return clearTimeout(timeoutInstnace)
port2.addListener('message', (response: 'ping' | WorkerResponse) => {
if (response === 'ping') return clearTimeout(timeoutInstnace)
const { error, value } = response
if (error) reject(error)
else resolve(value as Awaited<ReturnType<T>>)
clearTimeout(timeoutInstnace)
})

// --- Send the request payload to the target worker.
Expand Down

0 comments on commit 23b1f7c

Please sign in to comment.