[1.11.2] Add enhanced stack traces and a few more try-catch blocks#427
[1.11.2] Add enhanced stack traces and a few more try-catch blocks#427
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds enriched stack trace capture and augmentation utilities, integrating them into the NodeBaseConnection to improve error diagnostics.
- Introduces
getCurrentStackTraceandaddStackTracein the common error module - Wraps key error handlers in
NodeBaseConnectionto attach full call-site traces - Exports new utilities via the common package index
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/client-node/src/connection/node_base_connection.ts | Captures a baseline stack and wraps all error callbacks with augmented traces |
| packages/client-common/src/index.ts | Re-exports getCurrentStackTrace and addStackTrace alongside parseError |
| packages/client-common/src/error/index.ts | Updates error barrel to point to the new error.ts entrypoint |
| packages/client-common/src/error/error.ts | Defines getCurrentStackTrace, addStackTrace, and extends ClickHouseError |
Comments suppressed due to low confidence (2)
packages/client-common/src/error/error.ts:37
- The new utilities
getCurrentStackTraceandaddStackTracecurrently lack dedicated unit tests; adding tests would ensure these functions behave correctly across scenarios.
export function getCurrentStackTrace(): string {
packages/client-common/src/error/error.ts:53
- [nitpick]
addStackTraceis a public API but lacks JSDoc explaining its parameters, return value, and side effects; consider adding a doc comment for clarity.
export function addStackTrace<E extends Error>(err: E, stackTrace: string): E {
| } | ||
|
|
||
| function onTimeout(): void { | ||
| const err = addStackTrace(new Error('Timeout error.'), stackTrace) |
There was a problem hiding this comment.
Error constructor seems to have { cause } for changing the error messages https://nodejs.org/docs/latest-v18.x/api/errors.html#errorcapturestacktracetargetobject-constructoropt
slvrtrn
changed the title
Well, fair enough, let's add more unit tests. |
|
Summary
Added a new configuration option:
capture_enhanced_stack_trace; see the JS doc in the Node.js client package. Note that it is disabled by default due to a possible performance impact. For example:instead of previous:
Should be released as a patch version helping to debug #410