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

Question: Source mapping componentStack from error boundary in production #6667

Open
danielkcz opened this issue Mar 17, 2019 · 29 comments
Open

Comments

@danielkcz
Copy link

Lately, I am trying to use error boundaries to tackle unexpected errors more properly. I am also logging most of the errors to the Sentry service. I am sending the componentStack from componentDidMount there as well, but since it's minified, it feels fairly useless.

image

I am wondering what are my options if any. Since that output is pretty much just a string, the source mapping does not seem possible at any level.

@iansu
Copy link
Contributor

iansu commented Mar 17, 2019

You can upload your sourcemaps to Sentry. That might help in this situation.

@danielkcz
Copy link
Author

danielkcz commented Mar 17, 2019

@iansu That surely won't work because the componentStack is just a string, there is no structured information to feed source mapping mechanism. Have a look at this example (used to showcase different issue).

https://codesandbox.io/s/zw500lpm2p

@danielkcz danielkcz changed the title Question: Source mapping componentStack from error boundary Question: Source mapping componentStack from error boundary in production Mar 17, 2019
@atomicleads
Copy link

Source map upload definitely doesn't help. I have it uploaded, my stack trace is decoded properly, but componentStack is not.

@stale
Copy link

stale bot commented Apr 24, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Apr 24, 2019
@danielkcz
Copy link
Author

danielkcz commented Apr 24, 2019

Oh come on, I hate this little blue thing. Just because there is no viable answer yet, it shouldn't be stale :( This is a real issue. Can someone please tell that robot it's ok to keep it open?

@jeremygottfried
Copy link

Hi @iansu are the any updates on this issue? This is a significant for those of us who want to use error boundary to log errors.

@iansu
Copy link
Contributor

iansu commented Sep 3, 2019

There is no update on this. As far as I know no one is working on it.

@Tonyce
Copy link

Tonyce commented Oct 24, 2019

I have the same problem。Since React have given this component idea,I also expect a way.

@robertvansteen
Copy link
Contributor

Duplicate of #3753

@robertvansteen robertvansteen marked this as a duplicate of #3753 Oct 24, 2019
@danielkcz
Copy link
Author

@rovansteen You are right, it feels like duplicate, but I think it's still worth revisiting. In that linked issue there is no mention why that stack does not include line numbers. Something similar to a development version. It's totally fine if component names are minified, but if we had some location in a compiled bundle, it's another piece of information to ease the pain of finding the problem.

@robertvansteen
Copy link
Contributor

I agree, but as far as I understand that is something for https://github.com/facebook/react. Not much what we can do here.

@danielkcz
Copy link
Author

Well, if I understand it correctly, for development it's done by transform-react-jsx-source Babel plugin. That would mean it's more about CRA config than React.

I did found https://reactjs.org/docs/error-boundaries.html#component-stack-traces which mentions the plugin shouldn't be used in production, but not sure why really. Does it lower performance significantly? I haven't found any information regarding that.

@Tonyce
Copy link

Tonyce commented Oct 25, 2019

Well, I have got my solution. Could get the stack str from error. Thanks

@danielkcz
Copy link
Author

@Tonyce That's really nice of you that you want to share your solution ...

@sergei-startsev
Copy link

@FredyC In the same doc (https://reactjs.org/docs/error-boundaries.html#component-stack-traces) they also mentioned displayName property that can be used to get a proper component name displayed in the stack traces.

There's a babel plugin that allows to get a name with relative file path prefix in production.

@danielkcz
Copy link
Author

@sergei-startsev Ok that plugin is interesting.

@rovansteen Is it possibly something to consider to include in CRA?

@robertvansteen
Copy link
Contributor

I don't think we want to include something like this by default as it will increase the bundle size for everyone. So if something like this will be included it needs to be opt-in but I don't know if we want to go down that road of making the babel setup configurable.

I'll open this again as a proposal and see what other maintainers think of this.

@Tonyce
Copy link

Tonyce commented Oct 30, 2019

@FredyC

  ...
       const stackArr = body.split('\n');
        console.log(stackArr);
        const fileInfo = stackArr[1].split('/').pop();
        const [file, line, column] = fileInfo.split(':');
        console.log(file, line, column);
        const fileMap = await fsReadFile(`maps/${file}.map`, { encoding: 'utf8' });
        // console.log(fileMap);
        const consumer = await new sourceMap.SourceMapConsumer(fileMap);
        // doStuffWith(consumer);
        const result = consumer.originalPositionFor({ line: Number(line), column: Number(column.replace(')', '')) })
        console.log(result);
...
  xhr.send(e.stack); // e is the error
...

Is this demo clear ?

@danielkcz
Copy link
Author

danielkcz commented Oct 30, 2019

@Tonyce Thanks for sharing. So to have a correct error stack in production you are including extra 28kB in the bundle? Not exactly a slim solution. Also not sure what that fsReadFile is and how big it is.

Besides, it seems this will only map toward error stack trace that's mostly in React code, but not the actual component source files like seen in the other issue.

Nah, unless I am misunderstanding something here, then the above-mentioned plugin with auto attaching displayName still feels like a better solution given it only adds a couple of extra strings to bundle instead of the whole bunch of runtime code to tackle this.

@Tonyce
Copy link

Tonyce commented Nov 4, 2019

@FredyC fsReadFile just is fs.readFile in nodejs. in prod, i am not serve the mapfile, just keep them in the server side.

@danielkcz
Copy link
Author

danielkcz commented Nov 4, 2019

@Tonyce Oh, that's server-side, that makes more sense now. We are talking here about client-side caching of errors so please next time read carefully before you start announcing you got a solution :)

@rmehlinger
Copy link

To fix this, consider switching to Terser as your minification plugin. Terser provides an option to not mangle function names, which includes arrow functions, and also to not mangle class names: keep_fnames and keep_classnames. Both of these options take regexes. Since React component names are capitalized, you can pass the regex /[A-Z].+/and that should only hit your components (especially if you enforce camel casing for other functions with a linter). You'll get a slightly larger bundle size, but unless you're working on something that needs to be hyper-optimized it should be within tolerances.

Sample webpack config:

      new TerserJsPlugin({
        terserOptions: {
          output: {
            comments: false,
          },
          keep_classnames: true, // any classes are probably React Components anyway
          keep_fnames: /[A-Z].+/,
        },
        sourceMap: true,
      }),

@danielkcz
Copy link
Author

danielkcz commented Nov 5, 2019

@rmehlinger That's not an exactly flexible solution. Assuming all classes and functions are components is rather shallow thinking :)

Generally, people are rather sensitive to bundle size, so making it bigger to allow slightly better debugging is not going to pass through here.

Besides, we already have a suggestion for a smaller solution that attaches displayName to components only, so other things stay mangled as they should. But even that might not be justifiable to be enabled for everyone.

@rmehlinger
Copy link

rmehlinger commented Nov 5, 2019

Capitalized functions likely are components, though, especially as one can use a linter to enforce that other function names are lower-cased. Anyway... it's an option.

EDIT: Also, upon trying react-displayname-path, it appears to not work out of the box on functional components, though it does work on my class components. I suspect this has something to do with writing in TypeScript, but I'm not sure.

@jeremygottfried
Copy link

I like the idea of setting displayName property. For most of my cases, knowing the name of the main container component will suffice, so I think it's a bit overkill to set displayName of every component.

@osdiab
Copy link

osdiab commented May 6, 2020

Any updates on this issue? I'm interested in this as well.

@freemaths
Copy link

Also interested. In particular I log error stack traces from live but I need to map back to non-minimised line/column (I have the generated source maps).
I have tried using SourceMapConsumer from the https://github.com/mozilla/source-map package but it requires a .wasm file. Can create-react-app generate a wasm file or is there an alternative way to do the equivalent of:
consumer.originalPositionFor({ line: line, column: column})

@lijunle
Copy link

lijunle commented Jul 7, 2021

I have an idea. Instead of printing the little t in the component stack, could we print it with where the class is defined - the full JavaScript file path + line/column number.

That does not introduce any overhead in user React component code, maybe a little more code in React library code. It increases the weights of requests to logging system. But I think that should be fine - the weight here is helping debugging.


Update

Did more research and found React 17 has already this feature!

https://codepen.io/lijunle/pen/gOWrZdR?editors=0011

image

@bvaughn
Copy link
Contributor

bvaughn commented Jul 13, 2021

Also interested. In particular I log error stack traces from live but I need to map back to non-minimised line/column (I have the generated source maps).
I have tried using SourceMapConsumer from the mozilla/source-map package but it requires a .wasm file. Can create-react-app generate a wasm file or is there an alternative way to do the equivalent of:
consumer.originalPositionFor({ line: line, column: column})

@freemaths No opinion on whether or not this is a good idea, but you should be able to copy the wasm file from node_modules if you really want to do this:

cp ./node_modules/source-map/lib/mappings.wasm ./public/mappings.wasm

Then you can initialize it like:

SourceMapConsumer.initialize({'lib/mappings.wasm': '/mappings.wasm'});

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

No branches or pull requests