Repository navigation
fix(push): report noop and fetchMyEvents failures instead of leaving them unhandled - #243
Merged
EdamAme-x merged 1 commit intoOct 10, 2026
Conversation
…them unhandled `_OnPingCallback` fires `talk.noop()` every third ping (every 90 s) without a catch, and Conn calls `_OnPushResponse` without awaiting it, so a request that hits `config.timeout` rejects with an `AbortSignal.timeout` TimeoutError that nobody observes. Node and Deno treat that as an unhandled rejection and terminate the host process. Catch both and report them through `ConnManager.log` as `noop failed` / `fetchMyEvents failed`, keeping the connection alive. As in evex-dev#232 and evex-dev#239, the report is wrapped in try/catch because `log` fans out to user-supplied listeners and a throwing listener would otherwise bring the unhandled rejection back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two RPC calls in
ConnManagercan reject with nobody watching, and when they do the host process exits with an unhandled rejection instead of keeping the push connection alive._OnPingCallbackfiresthis.client.talk.noop().then(...)on every third ping (every 90 s with the default 30 s ping interval). It has no.catch(), andConn.onPacketReceivedcalls the callback synchronously and does not use its return value._OnPushResponseawaitsthis.client.square.fetchMyEvents(...)for every service 3 (Square) push frame, butConncalls the handler without awaiting it, so the returned promise is never observed either.Both requests carry
AbortSignal.timeout(config.timeout). When LINE is slow to answer, they reject with(a
DOMException,code: 23, whose stack has no frames from the caller because it comes from a timer). Since nothing catches it, Node and Deno both treat it as an unhandled rejection and terminate the process. We hit this in a long-running listener on Node: it ran fine for days and then exited on a single slownoopresponse.Fix
_OnPingCallback: add.catch()after the.then(...)and report the error asthis.log("noop failed", { error })(listeners receive[LEGY/PUSH] noop failed). The connection is left as is; the next ping sends anothernoop._OnPushResponse: wrap the body of the service 3 branch intry/catchand report asthis.log("fetchMyEvents failed", { error }).poll.sync.squareand the subscription are only updated after a successful fetch, so the next push frame retries from the same sync token. A throw fromreadThriftStructlands in the same catch.As in #232 and #239, the
logcall in each catch is wrapped in its owntry/catch, becauselogfans out to user-supplied listeners and a throwing listener would otherwise reject the handler and bring the unhandled rejection back.I used new log types rather than
LegyPusherErroron purpose: a failednooporfetchMyEventsis not a broken push connection, and consumers that treatLegyPusherErroras "the pusher is down" should not react to it.Testing
New
packages/linejs/base/push/connManager.test.ts, 5 cases against a stub client passed asnew ConnManager(client as never)(the same style asconn.test.ts), withtalk.noop/square.fetchMyEventsrejecting with the sameTimeoutErrorDOMException:_OnPingCallback(3)with a rejectingnoop→ one[LEGY/PUSH] noop failedcarrying the original error, no unhandled rejection._OnPushResponse(not awaited) with a rejectingfetchMyEvents→[LEGY/PUSH] fetchMyEvents failedcarrying the original error;poll.sync.squareunchanged and noupdate:syncdataemitted.fetchMyEventsstill enqueues the events onsqStream, updatespoll.sync.squareand the subscription id, emitsupdate:syncdataand logsSQ_fetchMyEvents.On
main@ ef6c3d9, cases 1–4 fail witherror: (in promise) TimeoutError: The operation was aborted due to timeout(Deno's sanitizer reports the unhandled rejection), and case 5 passes.deno test -Aat the repo root — 356 passed, 0 failed (351 onmain@ ef6c3d9)cd packages/linejs && deno publish --dry-run— successdeno checkclean on both touched files.deno fmt --checkis clean for the test file and for every line this PR changes inconnManager.ts; it still reports two pre-existing lines in that file (176 and 300 onmain), which I left alone to keep the diff focused.This change touches only
base/push/connManager.tsand adds its test.🤖 Generated with Claude Code