Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(vite): include id and stack in vite-node fallback error handler #7575

Merged
merged 6 commits into from
Sep 16, 2022

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Sep 15, 2022

πŸ”— Linked issue

ref nuxt/nuxt#14909

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This adds more info in the event of minimal error data. In the linked issue, error data is:

const errorData = {
  code: "ERR_MODULE_NOT_FOUND",
}

That's enough to thwart our formatter.

CleanShot 2022-09-15 at 23 09 56@2x

This PR adds stack + reason fallbacks, transforming it to:

CleanShot 2022-09-15 at 23 09 19@2x

Admittedly probably the stack isn't helpful in this case, but at least the error message tells us the module that couldn't be fetched. Alternatively, maybe we could directly throw the error if we only have a code in errorData?

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroe danielroe added dx vite 🍰 p2-nice-to-have Priority 2: nothing is broken but it's worth addressing labels Sep 15, 2022
@danielroe danielroe requested a review from pi0 September 15, 2022 22:12
@danielroe danielroe self-assigned this Sep 15, 2022
@netlify
Copy link

netlify bot commented Sep 15, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit c8bf713
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/6324bc24890e120008731618

@danielroe danielroe changed the title fix(vite): provide fallback stack + reason fix(vite): provide fallback reason Sep 16, 2022
@pi0
Copy link
Member

pi0 commented Sep 16, 2022

I've made #7589 for a better normalization approach. I think we can change this PR to make fatal errors (unhandled while normalizing) to include module id in the message. But reason is internal preserving it will be confusing.

@danielroe
Copy link
Member Author

Sure πŸ‘ Also happy to close this and we can do it all in your linked PR.

@pi0
Copy link
Member

pi0 commented Sep 16, 2022

I've double-checked now. Unless formatViteError fails (which is solved with other PR), we show 500 error (network error actually) with path to module id. That should only happen in rare cases of internal error. we could still show moduleId in fallback

@pi0 pi0 closed this Sep 16, 2022
@pi0 pi0 deleted the fix/error-fallback branch September 16, 2022 17:45
@pi0 pi0 restored the fix/error-fallback branch September 16, 2022 17:46
@pi0 pi0 reopened this Sep 16, 2022
@pi0
Copy link
Member

pi0 commented Sep 16, 2022

Improved fallback including id:

(note: this is not real! I had to make an intentional bug to the format error to produce above. just in case we missed something yet for normalization)

image

image

@pi0 pi0 changed the title fix(vite): provide fallback reason fix(vite): include module id in vite-node fallback errors Sep 16, 2022
@pi0 pi0 changed the title fix(vite): include module id in vite-node fallback errors fix(vite): include id and stack in vite-node fallback error handler Sep 16, 2022
@pi0 pi0 merged commit 5bc6544 into main Sep 16, 2022
@pi0 pi0 deleted the fix/error-fallback branch September 16, 2022 18:12
@pi0 pi0 mentioned this pull request Sep 20, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x dx 🍰 p2-nice-to-have Priority 2: nothing is broken but it's worth addressing vite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants