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

fix (remix-serve): fallback to default path if source maps aren't available when building stack trace #8446

Conversation

VHall1
Copy link
Contributor

@VHall1 VHall1 commented Jan 7, 2024

Another update to source map loading. This should resolve issues where people reported getting server crashes when they had source maps disabled or when errors were being thrown by 3rd party packages (since source maps aren't available for those). All this does really is check that the file exists before loading it, and returning null if it doesn't (using the original path as a fallback). Mostly based on the patch suggested here: #8309 (comment)

Testing:

3rd party error (tiny-invariant)
image

generic error
image

generic error after manually deleting source map
image

Closes: #8309

Copy link

changeset-bot bot commented Jan 7, 2024

🦋 Changeset detected

Latest commit: 27bf362

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/serve Patch
@remix-run/dev Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@VHall1 VHall1 changed the title Vhall1/return null if sourcemap is not available fix (remix-serve): fallback to default path if source maps aren't available when building stack trace Jan 7, 2024
@VHall1
Copy link
Contributor Author

VHall1 commented Jan 7, 2024

Not sure if this warrants a version bump, but happy to add a changeset if it does.

@VHall1 VHall1 marked this pull request as ready for review January 7, 2024 19:56
Copy link
Contributor

@brophdawg11 brophdawg11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@brophdawg11
Copy link
Contributor

Not sure if this warrants a version bump, but happy to add a changeset if it does.

yeah - since this changes source code, it should have a changeset. Would you mind pointing the PR to to dev as well?

@VHall1
Copy link
Contributor Author

VHall1 commented Jan 8, 2024

yep, sounds good to me! Will add the changeset and point to the correct branch later today.

@VHall1 VHall1 changed the base branch from main to dev January 8, 2024 23:34
@VHall1 VHall1 force-pushed the vhall1/return-null-if-sourcemap-is-not-available branch from 88f9ca3 to 9ba0cae Compare January 8, 2024 23:51
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 8, 2024

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@VHall1 VHall1 force-pushed the vhall1/return-null-if-sourcemap-is-not-available branch from 9ba0cae to fab00a0 Compare January 8, 2024 23:59
@VHall1
Copy link
Contributor Author

VHall1 commented Jan 9, 2024

sorry for the mess, should be pointing to dev now.

@brophdawg11 brophdawg11 merged commit 089f418 into remix-run:dev Jan 9, 2024
5 checks passed
Copy link
Contributor

github-actions bot commented Jan 9, 2024

🤖 Hello there,

We just published version 2.5.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 2.5.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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

Successfully merging this pull request may close these issues.

Error related to sourcemaps in production build
2 participants