Skip to content

fix: Handle custom error names in TraceKit #1237

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

Merged
merged 2 commits into from
Feb 21, 2018
Merged

Conversation

kamilogorek
Copy link
Contributor

Fixes #1209 (comment)

There's no RegExp that cannot be extended and still pass all unit tests

function computeStackTraceFromStackProp(ex) {
if (typeof ex.stack === 'undefined' || !ex.stack) return;

var chrome = /^\s*at (.*?) ?\(((?:file|https?|blob|chrome-extension|native|eval|webpack|<anonymous>|[a-z]:|\/).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i,
gecko = /^\s*(.*?)(?:\((.*?)\))?(?:^|@)((?:file|https?|blob|chrome|webpack|resource|\[native).*?|[^@]*bundle)(?::(\d+))?(?::(\d+))?\s*$/i,
gecko = /^\s*(.*?)(?:\((.*?)\))?(?:^|@)((?:file|https?|blob|chrome|webpack|resource).*?:\/.*?|\[native code\]|[^@]*bundle)(?::(\d+))?(?::(\d+))?\s*$/i,
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty sure blob is blob:foo and not blob://fo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Blob is blob:http://url/for/some/endpoint, which is matched by blob.*?:\/:) We have a test for it in place

Copy link
Contributor

@mitsuhiko mitsuhiko Feb 20, 2018

Choose a reason for hiding this comment

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

That depends on the browser. Current chrome adds the http origin to it, but blob:UUID is perfectly permissible and shows up in Firefox for instance.

You can try with (URL.createObjectURL(new Blob([new Int8Array()]))).toString()

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently blob urls are now supposed to always have an origin. Since they are rare I might be okay with that but please add a comment why blob is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added appropriate note and reformatted variables slightly so it's easier to move around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also made it two separate commits so it's easier to track precise changes

@kamilogorek kamilogorek force-pushed the tracekit-custom-errors branch 2 times, most recently from 14489ea to 1cc1915 Compare February 20, 2018 15:27
@kamilogorek kamilogorek force-pushed the tracekit-custom-errors branch from 1cc1915 to 845c83b Compare February 21, 2018 13:01
@kamilogorek kamilogorek merged commit 114c958 into master Feb 21, 2018
@kamilogorek kamilogorek deleted the tracekit-custom-errors branch February 21, 2018 13:10
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.

2 participants