Add asCause option to deserializeError - #118
Develop-KIM wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4f2a896e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const wrapAsCause = (error, stackStartFunction) => { | ||
| const wrappedError = newError(error.name); | ||
|
|
||
| for (const property of ['name', 'message', 'cause']) { |
There was a problem hiding this comment.
Preserve AggregateError.errors on wrapped errors
When deserializeError({name: 'AggregateError', errors: [...]}, {asCause: true}) is used, newError creates the wrapper with new AggregateError([]), and this loop only overwrites name, message, and cause. The returned error is still instanceof AggregateError, but error.errors is an empty array instead of the deserialized inner errors, so handlers that rely on the native AggregateError.errors API see no failures unless they know to inspect error.cause.errors.
Useful? React with 👍 / 👎.
The `asCause` wrapper is created via `newError`, which builds `AggregateError` with an empty `errors` array. Since the wrapper is `instanceof AggregateError`, copy `errors` onto it too so the native `AggregateError.errors` API sees the deserialized inner errors instead of an empty array (they were only reachable via `.cause.errors`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks, but I'm not interested in AI-generated PRs. I can also jsut use AI to generate it even better, so I don't really see the point of going back and forth with an AI. |
Closes #98.
Adds an
asCauseoption todeserializeError. When enabled, the deserialized error is wrapped as thecauseof a new error, so a thrown error carries a stack pointing at the current call site while the original (with its serialized stack) is kept on.cause:The wrapper is built with the resolved error constructor, so
instanceofstill works and it reuses the known-constructor handling. The stack is set viaError.captureStackTrace, so the wrapper's own frames don't leak into it.On the name: the thread didn't land on one. I went with
asCausesince that's what @fregante leaned toward, but it's a one-line change if you'd preferwraporcaptureStack— happy to switch.Docs and tests included;
npm test(xo + ava + tsd) passes.Written with AI assistance (Claude). I've reviewed the whole change and can walk through any of it.