ref(node): Allow node stack parser to work in browser context #5135
Merged
lforst merged 3 commits intogetsentry:7.xfrom May 27, 2022
Merged
ref(node): Allow node stack parser to work in browser context #5135lforst merged 3 commits intogetsentry:7.xfrom
lforst merged 3 commits intogetsentry:7.xfrom
Conversation
AbhiPrasad
approved these changes
May 19, 2022
AbhiPrasad
reviewed
May 19, 2022
| export { NodeClient } from './client'; | ||
| export { makeNodeTransport } from './transports'; | ||
| export { defaultIntegrations, init, lastEventId, flush, close, getSentryRelease } from './sdk'; | ||
| export { defaultIntegrations, init, defaultStackParser, lastEventId, flush, close, getSentryRelease } from './sdk'; |
Contributor
There was a problem hiding this comment.
Why not export nodeStackLineParser at the top level?
Collaborator
Author
There was a problem hiding this comment.
If we export from the top level and then import from there, will we not hit issues with bundlers trying to target the browser?
I just have painful memories of bundlers pulling in everything and erroring before they've even had a chance to do the treeshaking magic and remove the non-compatible code.
Contributor
There was a problem hiding this comment.
This is a fair point - maybe we just stick with this for now then.
Contributor
|
You might have to rebase because some lint failures sneaked into |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depending on configuration, Electron may have mixed Chrome/node.js stack traces and to decode these we need to use the node stack parser in the Electron renderers. Because the default Electron configuration does not have mixed Chrome/node.js context, for maximum compatibility, our renderer code must always be browser/bundler compatible which poses some issues when using the node.js stack parser.
The only node.js specific code in the stack parser fetches the
moduleusing node.js features (require.main.filename,global.process.cwd()). This PR makes thegetModulefunction an optional parameter and reorganises the file structure so we can import the required code without hitting anything node specific which will upset bundlers: