Fix issue #301 #302
Fix issue #301 #302
Conversation
There was a problem hiding this comment.
Pull request overview
Updates socket error handling to use Node.js string error codes (err.code) instead of numeric/negated errno values, addressing issue #301.
Changes:
- Switched retryable error code lists from numeric
os.constants.errno.*to string codes (e.g.,'ECONNRESET'). - Updated protocol error handler to compare
err.codedirectly (no negation). - Adjusted tests/helpers to return string error codes and removed
osdependency.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/errorHandling.js | Updates helper functions & docs to use string socket error codes; removes os usage. |
| lib/statsd.js | Compares retryable error codes against err.code directly. |
| lib/constants.js | Replaces numeric errno constants with string error codes; removes os import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/errorHandling.js
Outdated
| * Return system error code for a "bad connection" to a TCP socket (e.g. does not | ||
| * exist). Returns the string error code matching Node.js socket error.code. |
There was a problem hiding this comment.
The docstring says “system error code” but the function now returns a Node.js string error code (e.g., 'ECONNREFUSED'). To avoid confusion, update wording to consistently describe these as “Node.js error code strings” (or similar) rather than “system error code.”
test/errorHandling.js
Outdated
| * Return system error code for a "bad connection" to a UDS (e.g. does not | ||
| * exist). | ||
| * | ||
| * The value is negated because of the way errors are returned, e.g. by `libuv`. | ||
| * exist). Returns the string error code matching Node.js socket error.code. |
There was a problem hiding this comment.
The example “e.g. does not exist” is potentially misleading for UDS given the returned codes here are connection/refusal/reset style codes ('ECONNREFUSED', 'ECONNRESET') rather than a “missing path” style error (commonly 'ENOENT'). Consider adjusting the example to match the intended failing scenario (e.g., “server not accepting connections” / “peer reset”) so the comment aligns with the expected codes.
1 participant
Fixes #301