Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stack trace on custom ParserError #665

Merged
merged 1 commit into from
Apr 21, 2024
Merged

Fix stack trace on custom ParserError #665

merged 1 commit into from
Apr 21, 2024

Conversation

kewisch
Copy link
Owner

@kewisch kewisch commented Apr 15, 2024

I caught a ParserError with a weird stack, and it turns out the custom mangling we did before doesn't really cut it. With ES6+ we can simplify the instance variables.

I've only tested this in recent node and the firefox error console, both seem to contain a stack. The node stack even skips over the error class even without Error.captureStackTrace.

Does this look ok?

@kewisch kewisch requested a review from darktrojan April 15, 2024 10:07
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8687274538

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.003%) to 98.031%

Totals Coverage Status
Change from base Build 8687196681: -0.003%
Covered Lines: 9276
Relevant Lines: 9447

💛 - Coveralls

Copy link
Collaborator

@darktrojan darktrojan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc needs to be updated, but otherwise this seems okay to me.

@kewisch
Copy link
Owner Author

kewisch commented Apr 17, 2024

@darktrojan What would you like changed in the jsdoc? I think it is still accurate. I guess removing the @param so it rather inherits from Error maybe?

@darktrojan
Copy link
Collaborator

Yeah, that's what I had in mind. The @extends implies the @param. Today I am less convinced it matters. I'll approve the change and you can decide whether you want to keep it.

@kewisch kewisch merged commit 2f880d3 into main Apr 21, 2024
7 checks passed
@kewisch kewisch deleted the extends-error branch April 21, 2024 14:59
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