-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(browser): Split stack line parsers into individual functions and simplify further #4555
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
Conversation
Revert "More simplify" This reverts commit 84d194e. More working
For reference, the entire
Dropping default Opera support for v7 will save us another ~360 bytes. |
Sorry, this PR got a lot bigger than I wanted. It needed a lot of extra changes to get the tests passing. The good news is that the integration tests are working well! |
It looks like overall this results in 70 byte increase for ES5 min bundle and almost no change for ES6 min. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super busy, but one quick comment I saw, will come back and do a full review on monday!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks Tim!
stack-parsers.ts
TraceKitStackTrace
and instead just returnsStackFrame[]
exceptionFromStacktrace(computeStackTrace(e))
withexceptionFromError(e))
eventFromStacktrace(computeStackTrace(e))
witheventFromError(e))
Exception
which meant frames needed reversing andin_app
addingcreateStackParser
andStackLineParser
type to@sentry/utils
where there was already astacktrace.ts
prepareFramesForEvent
tostripSentryFramesAndReverse
and moved to utils too. This is nearly identical to the node.js code and means the parser now returns frames ready to go intoException
in_app
tocreateFrame
in stack parsers since everything browser is consideredin_app
stack.split('\n').slice(skipFirst)