Skip to content

fix(vite-node): top-level throw in module is not reported properly#6840

Merged
sheremet-va merged 4 commits intovitest-dev:mainfrom
vanaigr:main
Nov 5, 2024
Merged

fix(vite-node): top-level throw in module is not reported properly#6840
sheremet-va merged 4 commits intovitest-dev:mainfrom
vanaigr:main

Conversation

@vanaigr
Copy link
Contributor

@vanaigr vanaigr commented Nov 2, 2024

Description

A throw statement in a watched module can sometimes cause vite-node itself to crash (when trying to report that error), leading to the original error being lost.

Here is an example of this happening:
https://stackblitz.com/edit/vitest-dev-vitest-agi4oq?file=basic.ts

If you edit the file after the ready message is printed, vite-node will print the following error:

[vite-node] Failed to execute file: 
 TypeError: Cannot read properties of undefined (reading 'match')
    at warnFailedFetch (file:///home/projects/vitest-dev-vitest-agi4oq/node_modules/vite-node/dist/chunk-hmr.mjs:149:20)
    at fetchUpdate (file:///home/projects/vitest-dev-vitest-agi4oq/node_modules/vite-node/dist/chunk-hmr.mjs:137:7)
    at async queueUpdate (file:///home/projects/vitest-dev-vitest-agi4oq/node_modules/vite-node/dist/chunk-hmr.mjs:113:6)
    at async Module.handleMessage (file:///home/projects/vitest-dev-vitest-agi4oq/node_modules/vite-node/dist/chunk-hmr.mjs:164:7)

Explanation

This happens when a module is reloaded and throws something that is not an Error, e.g. a string.

This error propagates from runModule()

const fn = vm.runInThisContext(code, options)
await fn(...Object.values(context))
}

to warnFailedFetch(), where it fails at line 164 when it tries to call match on a non-existing message property of a string:

function warnFailedFetch(err: Error, path: string | string[]) {
if (!err.message.match('fetch')) {
console.error(err)
}

Solution

The easiest solution is to check if the error is actually an Error when printing before reading the message property.

I thought of wrapping the thrown object in an Error with cause in runModule(), but that would change how the error message is shown (since now the message and stacktrace would be from that error and not the original error).

@vanaigr
Copy link
Contributor Author

vanaigr commented Nov 2, 2024

Accidentally used main instead of the feature branch

Copy link
Contributor

@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.

Looks good! Just left a minor suggestion.

@hi-ogawa hi-ogawa changed the title Fix: top-level throw in module is not reported properly fix(vite-node): top-level throw in module is not reported properly Nov 4, 2024
@netlify
Copy link

netlify bot commented Nov 4, 2024

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1b0e0e5
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/672832e2f6aa740008d80b2f
😎 Deploy Preview https://deploy-preview-6840--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sheremet-va sheremet-va merged commit cf0cbf6 into vitest-dev:main Nov 5, 2024
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Nov 13, 2024
##### [v2.1.5](https://github.com/vitest-dev/vitest/releases/tag/v2.1.5)

#####    🚀 Features

-   **experimental**: Expose vite on the test project as a separate field  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(bcc59)</samp>](vitest-dev/vitest@bcc59088)

#####    🐞 Bug Fixes

-   `dangerouslyIgnoreUnhandledErrors` without base reporter  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6808 [<samp>(0bf0a)</samp>](vitest-dev/vitest@0bf0ab00)
-   Capture `unhandledRejection` even when base reporter is not used  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6812 [<samp>(8878b)</samp>](vitest-dev/vitest@8878b04d)
-   Don't change the working directory when loading workspace projects  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6811 [<samp>(f0aea)</samp>](vitest-dev/vitest@f0aeaca8)
-   Remove `sequence.concurrent` from the `RuntimeConfig` type  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6880 [<samp>(6af73)</samp>](vitest-dev/vitest@6af73d93)
-   Stop the runner before restarting, restart on workspace config change  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6859 [<samp>(b01df)</samp>](vitest-dev/vitest@b01df47d)
-   Don't rerun on Esc or Ctrl-C during watch filter  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6895 [<samp>(98f76)</samp>](vitest-dev/vitest@98f76ea7)
-   Print ssrTransform error  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6885 [<samp>(4c96c)</samp>](vitest-dev/vitest@4c96cce7)
-   Throw an error and a warning if `.poll`, `.element`, `.rejects`/`.resolves`, and `locator.*` weren't awaited  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6877 [<samp>(93b67)</samp>](vitest-dev/vitest@93b67c24)
-   **browser**:
    -   Don't process the default css styles  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6861 [<samp>(0d67f)</samp>](vitest-dev/vitest@0d67f04b)
    -   Support non US key input  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6873 [<samp>(5969d)</samp>](vitest-dev/vitest@5969d8da)
    -   Stop the browser rpc when the pool is closed  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6858 [<samp>(9a0c9)</samp>](vitest-dev/vitest@9a0c93d7)
    -   Init browsers eagerly when tests are running  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6876 [<samp>(417bd)</samp>](vitest-dev/vitest@417bdb42)
-   **coverage**:
    -   Report uncovered files when re-run by `enter` or `'a'`  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6848 [<samp>(487c8)</samp>](vitest-dev/vitest@487c80ae)
-   **ui**:
    -   Remove crossorigin attributes for same origin assets  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6883 [<samp>(6e793)</samp>](vitest-dev/vitest@6e793c64)
-   **vite-node**:
    -   Improve esm check to decide external  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6816 [<samp>(7e1fa)</samp>](vitest-dev/vitest@7e1faf3d)
    -   Top-level throw in module is not reported properly  -  by [@vanaigr](https://github.com/vanaigr) and [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6840 [<samp>(cf0cb)</samp>](vitest-dev/vitest@cf0cbf6a)
-   **vitest**:
    -   Use fetch() implementation from happy-dom  -  by [@capricorn86](https://github.com/capricorn86) in vitest-dev/vitest#6879 [<samp>(3c56f)</samp>](vitest-dev/vitest@3c56feb4)

#####    🏎 Performance

-   Limit workspace project concurrency to available cores  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6869 [<samp>(f4c04)</samp>](vitest-dev/vitest@f4c04ab6)

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.4...v2.1.5)
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Nov 13, 2024
##### [v2.1.5](https://github.com/vitest-dev/vitest/releases/tag/v2.1.5)

#####    🚀 Features

-   **experimental**: Expose vite on the test project as a separate field  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(bcc59)</samp>](vitest-dev/vitest@bcc59088)

#####    🐞 Bug Fixes

-   `dangerouslyIgnoreUnhandledErrors` without base reporter  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6808 [<samp>(0bf0a)</samp>](vitest-dev/vitest@0bf0ab00)
-   Capture `unhandledRejection` even when base reporter is not used  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6812 [<samp>(8878b)</samp>](vitest-dev/vitest@8878b04d)
-   Don't change the working directory when loading workspace projects  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6811 [<samp>(f0aea)</samp>](vitest-dev/vitest@f0aeaca8)
-   Remove `sequence.concurrent` from the `RuntimeConfig` type  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6880 [<samp>(6af73)</samp>](vitest-dev/vitest@6af73d93)
-   Stop the runner before restarting, restart on workspace config change  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6859 [<samp>(b01df)</samp>](vitest-dev/vitest@b01df47d)
-   Don't rerun on Esc or Ctrl-C during watch filter  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6895 [<samp>(98f76)</samp>](vitest-dev/vitest@98f76ea7)
-   Print ssrTransform error  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6885 [<samp>(4c96c)</samp>](vitest-dev/vitest@4c96cce7)
-   Throw an error and a warning if `.poll`, `.element`, `.rejects`/`.resolves`, and `locator.*` weren't awaited  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6877 [<samp>(93b67)</samp>](vitest-dev/vitest@93b67c24)
-   **browser**:
    -   Don't process the default css styles  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6861 [<samp>(0d67f)</samp>](vitest-dev/vitest@0d67f04b)
    -   Support non US key input  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6873 [<samp>(5969d)</samp>](vitest-dev/vitest@5969d8da)
    -   Stop the browser rpc when the pool is closed  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6858 [<samp>(9a0c9)</samp>](vitest-dev/vitest@9a0c93d7)
    -   Init browsers eagerly when tests are running  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6876 [<samp>(417bd)</samp>](vitest-dev/vitest@417bdb42)
-   **coverage**:
    -   Report uncovered files when re-run by `enter` or `'a'`  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6848 [<samp>(487c8)</samp>](vitest-dev/vitest@487c80ae)
-   **ui**:
    -   Remove crossorigin attributes for same origin assets  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6883 [<samp>(6e793)</samp>](vitest-dev/vitest@6e793c64)
-   **vite-node**:
    -   Improve esm check to decide external  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6816 [<samp>(7e1fa)</samp>](vitest-dev/vitest@7e1faf3d)
    -   Top-level throw in module is not reported properly  -  by [@vanaigr](https://github.com/vanaigr) and [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6840 [<samp>(cf0cb)</samp>](vitest-dev/vitest@cf0cbf6a)
-   **vitest**:
    -   Use fetch() implementation from happy-dom  -  by [@capricorn86](https://github.com/capricorn86) in vitest-dev/vitest#6879 [<samp>(3c56f)</samp>](vitest-dev/vitest@3c56feb4)

#####    🏎 Performance

-   Limit workspace project concurrency to available cores  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6869 [<samp>(f4c04)</samp>](vitest-dev/vitest@f4c04ab6)

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.4...v2.1.5)
spamshaker pushed a commit to spamshaker/vitest that referenced this pull request Jun 30, 2025
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

Successfully merging this pull request may close these issues.

3 participants