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(ssr): use optional chaining to prevent "undefined is not an object" happening in ssrRewriteStacktrace #19612

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

saintnoodle
Copy link
Contributor

Description

This PR handles a case where varName passed from a RegExp match on stacktrace lines is undefined and causes ssrRewriteStacktrace to fail on varName.trim().

This bug was encounted in the Vite dev server running with Bun via the command bunx --bun vite.

I have a reproduction which demonstrates the error in a simple manner. There is also a patch for Vite on the branch "fix-patch" which demonstrates the fixed behaviour.

I'm not the first to encounter this according to this issue over at Bun.

Explaination

Upon inspecting the stack traces and comparing them between Node and Bun, the first line of the stack trace tends to contain "eval", which means the function safely accesses varName.trim() and continues to completion. However, in Bun, varName is undefined which causes the function to fail when it tries to access .trim() where it doesn't exist.

If it weren't for modules outside of the module graph being skipped after moduleGraph.getModuleById(), is undefined failure would be common as anonymous entries in the stacktrace usually occur.

The existance of if (!trimmedVarName ... indicates to me that this was expected, but trimmedVarName can't be undefined because of the lack of optional chaining since the function fails if the value is not a string before undefined can be assigned.

Bun stacktrace:

Error: Anonymous Error
    at C:/Users/{username}/Projects/vite-ssr-repro/app/routes/anonymous-error.tsx:7:10
    at loader (C:/Users/{username}/Projects/vite-ssr-repro/app/routes/anonymous-error.tsx:8:4)
    at C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:9174:21
    at callRouteHandler (C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:9173:32)
    at C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:9287:24
    at C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:9263:43
    at C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:3893:90
    at C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:3891:38
    at runHandler (C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:3896:42)
    at C:\Users\{username}\Projects\vite-ssr-repro\node_modules\react-router\dist\development\index.js:3943:21

Node stacktrace:

Error: Anonymous Error
    at eval (C:/Users/{username}/Projects/vite-ssr-node/app/routes/anonymous-error.tsx:7:11)
    at loader (C:/Users/{username}/Projects/vite-ssr-node/app/routes/anonymous-error.tsx:8:5)
    at callRouteHandler (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:9026:22)
    at commonRoute.loader (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:9139:25)
    at actualHandler (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3734:14)
    at file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3745:91
    at runHandler (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3750:7)
    at callLoaderOrAction (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3795:22)
    at Object.resolve (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3690:27)
    at file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3565:62
    at Array.map (<anonymous>)
    at defaultDataStrategy (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3565:49)
    at callDataStrategyImpl (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3705:23)
    at callDataStrategy (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3117:25)
    at loadRouteData (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:3084:25)
    at queryImpl (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:2900:26)
    at Object.query (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:2764:24)
    at handleDocumentRequest (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:9758:40)
    at requestHandler (file:///C:/Users/{username}/Projects/vite-ssr-node/node_modules/react-router/dist/development/chunk-K6CSEXPM.mjs:9684:24)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at nodeHandler (C:\Users\{username}\Projects\vite-ssr-node\node_modules\@react-router\dev\dist\vite.js:2932:30)
    at C:\Users\{username}\Projects\vite-ssr-node\node_modules\@react-router\dev\dist\vite.js:2938:17

Copy link
Collaborator

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

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

I think adding this small safety measure is okay, but the reproduction might be somewhat specific to react-router, which is using vite-node's source map support at the same time. https://github.com/remix-run/react-router/blob/796e9ae10f74ba453b738f9d80049885f24caccf/packages/react-router-dev/vite/vite-node.ts#L47-L49

Normally bun's error seems to have <anonymous> in the stack like below, but vite-node's Error.prepareStackTrace probably strips it out somehow, which might be the actual part of bun's node incompatibility.

Error: __TEST_ERROR__
    at <anonymous> (/xxx/src/test.tsx:5:20)
    at test (/xxx/src/test.tsx:6:5)
    at <anonymous> (/xxx/repro.js:8:9)
    at processTicksAndRejections (native:7:39)

@saintnoodle
Copy link
Contributor Author

I knew there was something going on further up the chain, but I couldn't quite figure it out. This makes things a lot clearer, thanks for pointing that out.

Copy link
Collaborator

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

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

My best guess is that bun might have a different behavior in CallSite.getFunctionName https://github.com/vitest-dev/vitest/blob/0c2924b7aeb7eaf1b1fa0e78d2da76a7f3197b50/packages/vite-node/src/source-map-handler.ts#L404-L411 If someone can debug this, it would be better to be reported on bun, but adding optional chain looks fine for me.


Or maybe that's not it. Node has eval but Bun has <anonymous>, which probably makes vite-node to do a different thing 🤔

@saintnoodle
Copy link
Contributor Author

saintnoodle commented Mar 10, 2025

<anonymous> only appears to display in the console and on CallSite.toString(). On anonymous stack entries, CallSite.getFunctionName() returns undefined. I don't think I'm smart enough to understand the why of all this, but these are observations I've made poking around. The main thing I'm confused about is why eval is reported in Node and not in Bun.

@sapphi-red sapphi-red added feat: ssr p2-edge-case Bug, but has workaround or limited in scope (priority) labels Mar 10, 2025
@sapphi-red sapphi-red changed the title fix(vite): optional chaining to prevent error if varName is undefined fix(ssr): use optional chaining to prevent "undefined is not an object" happening in ssrRewriteStacktrace Mar 10, 2025
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

Putting Bun's Node compat bug aside, I think we should merge this PR as the regex allows varName to be undefined and the code should have been written like this.

@sapphi-red sapphi-red merged commit 4309755 into vitejs:main Mar 10, 2025
18 checks passed
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Mar 14, 2025
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.2.1 | 6.2.2 |


## [v6.2.2](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small622-2025-03-14-small)

-   fix: await client buildStart on top level buildStart ([#19624](vitejs/vite#19624)) ([b31faab](vitejs/vite@b31faab)), closes [#19624](vitejs/vite#19624)
-   fix(css): inline css correctly for double quote use strict ([#19590](vitejs/vite#19590)) ([d0aa833](vitejs/vite@d0aa833)), closes [#19590](vitejs/vite#19590)
-   fix(deps): update all non-major dependencies ([#19613](vitejs/vite#19613)) ([363d691](vitejs/vite@363d691)), closes [#19613](vitejs/vite#19613)
-   fix(indexHtml): ensure correct URL when querying module graph ([#19601](vitejs/vite#19601)) ([dc5395a](vitejs/vite@dc5395a)), closes [#19601](vitejs/vite#19601)
-   fix(preview): use preview https config, not server ([#19633](vitejs/vite#19633)) ([98b3160](vitejs/vite@98b3160)), closes [#19633](vitejs/vite#19633)
-   fix(ssr): use optional chaining to prevent "undefined is not an object" happening in \`ssrRewriteStac ([4309755](vitejs/vite@4309755)), closes [#19612](vitejs/vite#19612)
-   feat: show friendly error for malformed `base` ([#19616](vitejs/vite#19616)) ([2476391](vitejs/vite@2476391)), closes [#19616](vitejs/vite#19616)
-   feat(worker): show asset filename conflict warning ([#19591](vitejs/vite#19591)) ([367d968](vitejs/vite@367d968)), closes [#19591](vitejs/vite#19591)
-   chore: extend commit hash correctly when ambigious with a non-commit object ([#19600](vitejs/vite#19600)) ([89a6287](vitejs/vite@89a6287)), closes [#19600](vitejs/vite#19600)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ssr p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants