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

Request: Always resolve source maps on Errors #729

Closed
huw opened this issue Oct 30, 2023 · 4 comments
Closed

Request: Always resolve source maps on Errors #729

huw opened this issue Oct 30, 2023 · 4 comments

Comments

@huw
Copy link
Contributor

huw commented Oct 30, 2023

For this worker:

export default {
  fetch: async () => {
    console.error(new Error("Pretty error test"));
    return new Response("OK");
  },
};

I would expect the console.error call to print the error with a source mapped stack trace (like it does if I throw the error), but it doesn’t. I hunted through the Miniflare code and found that we do construct source maps for thrown errors, but don’t override prepareStackTrace by default.

I tried removing the line Error.prepareStackTrace = originalPrepareStackTrace; in getSourceMapper() and calling getSourceMapper() on launch, but it didn’t seem to override prepareStackTrace. Nor did passing NODE_OPTIONS=--source-map-support to wrangler dev.

I’m not expecting this to be fixed anytime soon (although I’ll happily take pointers on how to fix it myself!), but I wanted to raise the issue so it’s tracked somewhere. Cheers again for all the hard work on this repo, it’s not unappreciated ❤️

@frandiox
Copy link
Contributor

I might be mistaken but I think that, in latest Miniflare 3, the error comes already stringified from Workerd and Miniflare just prints it without modifying the stack trace. Somewhere around here. Maybe that's the place to add sourcemaps 🤔

@mrbbot
Copy link
Contributor

mrbbot commented Oct 30, 2023

Hey! 👋 Yep, in Miniflare 3 (which I'm assuming this issue is for), we just see a string from workerd here:

stderr.on("line", (data) => console.error(red(data)));

This is the same problem as logging stacks to the console (i.e. console.log(new Error().stack));
There was a PR to workers-sdk to address this issue (cloudflare/workers-sdk#3739). We're planning on addressing that soon. 👍

@huw
Copy link
Contributor Author

huw commented Oct 30, 2023

Can’t believe I didn’t see that—thank you! I’ll leave it to you to whether you wanna close this issue now or after that ships :)

@mrbbot
Copy link
Contributor

mrbbot commented Nov 7, 2023

Hey! 👋 Thanks for raising this issue. I was going to transfer this to workers-sdk as that's the new home for Miniflare 3, but considering we already have that PR and this is on our internal issue tracker, I'm going to close this.

@mrbbot mrbbot closed this as completed Nov 7, 2023
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

No branches or pull requests

3 participants