fix(sync): guard against parser errors in UDP message listener - #114
Merged
Conversation
Wrap NtpPacketParser.parse() in try/catch inside the dgram message listener so a parse failure rejects the pending promise cleanly instead of escaping as an uncaughtException. This matters once the parser lands input validation (it will throw TypeError on non-48-byte or non-Buffer inputs); without this guard a single malformed UDP response would crash the host Node process. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
NtpPacketParser.parse()inside the dgram"message"listener in a try/catch so a parse failure rejects the pending promise cleanly instead of escaping the callback.ntp-packet-parserstays at^0.5.0. This is the consumer-side guard, landed independently ahead of the parser's input-validation change.hasFinished = trueand callsreject(err), preserving the existing invariants aroundhasFinished,timeoutHandler, andclient.close().Context
An upcoming
ntp-packet-parserrelease will add input validation toNtpPacketParser.parse()— it will throwTypeErroron non-48-byte inputs or non-Bufferarguments. Without this guard, that throw would escape the dgram listener callback and surface asuncaughtExceptionon the Node process. Worse, because the listener is the only path that clearstimeoutHandler/ setshasFinished, the outer promise would hang untilreplyTimeoutfires — and on Node processes without a globaluncaughtExceptionhandler, the process would crash outright.With this guard, a malformed UDP response produces a rejected promise that the existing retry loop in
collectSamplesalready handles (it catches via.catch((e) => e)and filtersErrorresults out ofntpResults).Test plan
There are no existing tests in this repo, so
yarn build(tsc) is the only automated gate, and it passes.Manual repro against a local UDP server that returns a malformed response:
Note
This PR intentionally does NOT bump the
ntp-packet-parserdependency. The parser-side change (input validation) will land in a separate release; this consumer-side guard is defensive and safe to ship first.🤖 Generated with Claude Code