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

🐛 BUG: Wrangler 3 and Wrangler 2 both point to incorrect source code in stack traces #3383

Closed
BillBrower-Shopify opened this issue May 31, 2023 · 7 comments · Fixed by #4423
Assignees
Labels
bug Something that isn't working

Comments

@BillBrower-Shopify
Copy link

Which Cloudflare product(s) does this pertain to?

Wrangler

What version of Wrangler are you using?

3.0.0

What operating system are you using?

Mac

Describe the Bug

Both Wrangler 2.15.0 with --experimental-local and 3.0.0 without any --local or --experimental-local flag point to incorrect files and line numbers when displaying stack traces. That is stack traces end up looking like this:

TypeError: Cannot read properties of undefined (reading 'propertyName')
    at ComponentName (index.js:292996:187)
    at renderWithHooks (index.js:98065:24)
    at renderIndeterminateComponent (index.js:98122:24)
    at renderElement (index.js:98278:15)
    at renderNodeDestructiveImpl (index.js:98382:17)
    at renderNodeDestructive (index.js:98362:22)
    at renderIndeterminateComponent (index.js:98158:15)
    at renderElement (index.js:98278:15)
    at renderNodeDestructiveImpl (index.js:98382:17)
    at renderNodeDestructive (index.js:98362:22)
@BillBrower-Shopify BillBrower-Shopify added the bug Something that isn't working label May 31, 2023
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk May 31, 2023
@penalosa
Copy link
Contributor

Thanks for reporting this! Could you link a reproduction repo so we can investigate more? What sort of stack traces are you expecting to see?

@penalosa penalosa added the needs reproduction Needs reproduction from OP label Jun 16, 2023
@BillBrower-Shopify
Copy link
Author

@penalosa I just created this minimal reproduction repository https://github.com/BillBrower-Shopify/wrangler-source-map-reproduction

The stack trace from a sample indie-stack Remix app running on Node with an error thrown in the index route looks like this:

Error: This is the error I'm trying to reproduce
    at Index (/project/app/routes/_index.tsx:9:9)
    at renderWithHooks (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:5724:16)
    at renderIndeterminateComponent (/projectnode_modules/react-dom/cjs/react-dom-server.node.development.js:5797:15)
    at renderElement (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6012:7)
    at renderNodeDestructiveImpl (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6170:11)
    at renderNodeDestructive (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6142:14)
    at renderIndeterminateComponent (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:5851:7)
    at renderElement (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6012:7)
    at renderNodeDestructiveImpl (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6170:11)
    at renderNodeDestructive (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6142:14)
    at renderContextProvider (/projectnode_modules/react-dom/cjs/react-dom-server.node.development.js:5986:3)
    at renderElement (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6083:11)
    at renderNodeDestructiveImpl (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6170:11)
    at renderNodeDestructive (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6142:14)
    at renderIndeterminateComponent (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:5851:7)
    at renderElement (/project/node_modules/react-dom/cjs/react-dom-server.node.development.js:6012:7)

An equivalent app run inside of Wrangler with an error thrown in the index route produces a stack trace that looks like this:

Error: This is the error I'm trying to reproduce
    at Index (index.js:28390:13)
    at renderWithHooks (index.js:23919:24)
    at renderIndeterminateComponent (index.js:23976:23)
    at renderElement (index.js:24132:15)
    at renderNodeDestructiveImpl (index.js:24236:17)
    at renderNodeDestructive (index.js:24216:22)
    at renderIndeterminateComponent (index.js:24012:15)
    at renderElement (index.js:24132:15)
    at renderNodeDestructiveImpl (index.js:24236:17)
    at renderNodeDestructive (index.js:24216:22)

@lrapoport-cf lrapoport-cf added awaiting Cloudflare response Awaiting response from workers-sdk maintainer team bucket:shallot and removed needs reproduction Needs reproduction from OP labels Jul 12, 2023
@mrbbot
Copy link
Contributor

mrbbot commented Jul 20, 2023

Hey! 👋 The problem here is that Remix is catching the error inside the worker. If you create a TypeScript worker like...

export default <ExportedHandler>{
  async fetch() {
    try {
       throw new Error("Ooops!");
    } catch (e) {
      return new Response(e.stack);
    }
  }
}

...you'll see the stack trace refers to the built JavaScript file instead.

Unfortunately, workerd doesn't support source mapping natively yet, so we have to do this outside the worker in Wrangler/Miniflare. This only works if the error is "uncaught", and bubbles up outside the fetch handler. If you replaced the above example with...

export default <ExportedHandler>{
  async fetch() {
     throw new Error("Ooops!");
  }
}

...that should source map correctly.

I believe we're planning to add support to workerd for this, so I'll keep this issue open to track that. In the meantime, I wonder if there's a way to disable Remix's own error page? Wrangler has its own pretty-error page that serves a similar purpose.

@BillBrower-Shopify
Copy link
Author

Thanks @mrbbot! That makes sense, I'll follow up with the Remix project and see if there's a way to configure that for development purposes and then have the worker render an error page from Wrangler instead

@BillBrower-Shopify
Copy link
Author

@mrbbot The Remix team said there is no way to configure Remix to bubble the errors up to Wrangler instead of catching them

@mrbbot
Copy link
Contributor

mrbbot commented Oct 30, 2023

Hey! 👋 Just to keep you updated, we're planning to fix this issue as part of #3739 (comment) and cloudflare/miniflare#729 soon. 👍

@mrbbot mrbbot removed the awaiting Cloudflare response Awaiting response from workers-sdk maintainer team label Oct 30, 2023
@mrbbot mrbbot moved this from Untriaged to Backlog in workers-sdk Oct 30, 2023
@mrbbot mrbbot added blocked Blocked on other work and removed blocked Blocked on other work labels Nov 14, 2023
@mrbbot mrbbot moved this from Backlog to In Progress in workers-sdk Nov 14, 2023
@mrbbot
Copy link
Contributor

mrbbot commented Nov 16, 2023

Hey! 👋 As an update, we have a draft PR up that should fix this (#4423), but it's blocked on #4341 going in first. Once that's landed, we should be able to rebase and merge the fix. 👍

@lrapoport-cf lrapoport-cf removed the blocked Blocked on other work label Nov 27, 2023
@mrbbot mrbbot moved this from In Progress to In Review in workers-sdk Nov 28, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in workers-sdk Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants