Skip to content

refactor: do not overwrite the error stack in json parser#604

Closed
bjohansebas wants to merge 1 commit intoexpressjs:masterfrom
bjohansebas:stack
Closed

refactor: do not overwrite the error stack in json parser#604
bjohansebas wants to merge 1 commit intoexpressjs:masterfrom
bjohansebas:stack

Conversation

@bjohansebas
Copy link
Copy Markdown
Member

to simplify the JSON parser a bit

@jonchurch
Copy link
Copy Markdown
Member

I didn't realize this stack manipulation was here tbh. Likely no one is matching on stacktraces from body-parser, but with things that have potentially been part of someone's monitoring stack for 8 years, Im hesitant to change it outside a major.

I'd rather we keep this how it is until we decide to change error reporting in this package in a more meaningful way ala using error.cause as a better way to enrich errors while keeping the stack traceable

@jonchurch jonchurch closed this Apr 8, 2025
@Phillip9587
Copy link
Copy Markdown
Member

@bjohansebas Can you please open an issue so we don't loose it for the next major version?

@bjohansebas
Copy link
Copy Markdown
Member Author

i'd say this is a clean change, since it only modifies the error message, which is already being handled in the error.message part, and it wouldn't introduce any breaking changes.

and it does make sense to migrate it to error.cause later, i’ll open an issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants