fix: handle error from pool.run #1348

sapphi-red · 2022-05-21T12:31:22Z

When an error was thrown from pool.run, vitest finished without any error messages.
This PR handles that error.

I found this while implementing vitejs/vite#8049.

Before

 RUN  v0.12.8 D:/documents/GitHub/vite

····*···---******************************---------··········***····································******···*********·····*·**··········***

There was no error message.

After

 RUN  v0.12.8 D:/documents/GitHub/vite

stderr | unknown test
"default" is imported from external module "path" but never used in "playground-temp/ssr-vue/src/entry-server.js".

················································---································································---------··················-----------··············································--·*··························-··································-****·············································································-------··---····································----------·······················································*·············************··

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
 FAIL  playground/react-emotion/__tests__/react.spec.ts [ playground/react-emotion/__tests__/react.spec.ts ]
Error: Cannot find package '@emotion/babel-plugin' imported from D:\documents\GitHub\vite\babel-virtual-resolve-base.js
 ❯ new NodeError node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:2669:5
 ❯ packageResolve node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:3325:9
 ❯ moduleResolve node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:3359:18
 ❯ defaultResolve node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:3398:13
 ❯ node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:3421:14
 ❯ asyncGeneratorStep node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:63:103
 ❯ _next node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:65:194
 ❯ node_modules/.pnpm/@babel+core@7.17.10/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:65:364

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯Serialized Error: {
  "code": "PLUGIN_ERROR",
  "hook": "transform",
  "id": "D:/documents/GitHub/vite/playground-temp/react-emotion/index.html?html-proxy&index=0.js",
  "plugin": "vite:react-babel",
  "pluginCode": "ERR_MODULE_NOT_FOUND",
  "watchFiles": [
    "D:/documents/GitHub/vite/playground-temp/react-emotion/index.html",
    "vite/modulepreload-polyfill",
    "D:/documents/GitHub/vite/playground-temp/react-emotion/index.html?html-proxy&index=0.js",
  ],
}
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
Test Files  1 failed | 57 passed | 3 skipped (64)
     Tests  422 passed | 47 skipped (487)
      Time  22.63s (in thread 174.57s, 12.97%)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Vitest caught 1 unhandled error during the test run. This might cause false positive tests.
Please, resolve all the errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯AggregateError: 
 ❯ Object.runTests ../../../../file:/D:/documents/GitHub/vitest/packages/vitest/dist/vendor-cli-api.bcfd31a5.js:8250:15
 ❯ processTicksAndRejections ../../../../node:internal/process/task_queues:96:5        
 ❯ async ../../../../file:/D:/documents/GitHub/vitest/packages/vitest/dist/vendor-cli-api.bcfd31a5.js:10440:9
 ❯ async Vitest.runFiles ../../../../file:/D:/documents/GitHub/vitest/packages/vitest/dist/vendor-cli-api.bcfd31a5.js:10450:12
 ❯ async Vitest.start ../../../../file:/D:/documents/GitHub/vitest/packages/vitest/dist/vendor-cli-api.bcfd31a5.js:10377:5
 ❯ async startVitest ../../../../file:/D:/documents/GitHub/vitest/packages/vitest/dist/vendor-cli-api.bcfd31a5.js:11113:5
 ❯ async start ../../../../file:/D:/documents/GitHub/vitest/packages/vitest/dist/cli.js:665:9
js:661:3

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯Serialized Error: {
  "errors": [
    [Error: ENOENT: no such file or directory, open 'D:\documents\GitHub\vite\playground-temp\js-sourcemap\dist\assets\index.704baece.js'],
    [Error: ENOENT: no such file or directory, open 'D:\documents\GitHub\vite\playground-temp\worker\dist\iife\index.html'],
    [Error: ENOENT: no such file or directory, open 'D:\documents\GitHub\vite\playground-temp\worker\dist\iife-sourcemap\assets\my-worker.b928aeec.js'],
  ],
}
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

It shows as a Unhandled Error but it is now much better than no error message.
To be honest, I don't know where it should be handled.

netlify · 2022-05-21T12:31:28Z

✅ Deploy Preview for vitest-dev ready!

Name	Link
🔨 Latest commit	`7478246`
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/6288f18174ba8f00084b8a48
😎 Deploy Preview	https://deploy-preview-1348--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

sheremet-va

CI is not happy :S

packages/vitest/src/node/pool.ts

packages/vitest/src/utils/index.ts

sheremet-va · 2022-05-21T12:49:01Z

Also, we are calling this function in

vitest/packages/vitest/src/node/core.ts

Line 220 in a23fb58

await this.pool.runTests(files, invalidates)

Which has a try/catch 🤔

sapphi-red · 2022-05-21T13:48:04Z

Also, we are calling this function in

vitest/packages/vitest/src/node/core.ts

Line 220 in a23fb58

await this.pool.runTests(files, invalidates)

Which has a try/catch 🤔

When an error happens here, these two .close() will not be called.

vitest/packages/vitest/src/node/pool.ts

Line 94 in 478465a

await pool.run(data, { transferList: [workerPort], name })

vitest/packages/vitest/src/node/pool.ts

Lines 95 to 96 in 478465a

    
           port.close() 
        
           workerPort.close()

I don't know what happens if it is not called, but it seems it needs to be called.

sheremet-va · 2022-05-21T13:51:53Z

When an error happens here, these two .close() will not be called.

I guess they need to be called, yes (so we need to keep your try/finally), but do we need to throw AggregateError then? Or you think it's better for overall DX? If so, I think we might need a better error message showing where errors came from.

sapphi-red · 2022-05-21T13:55:47Z

If I use Promise.all instead of Promise.allSettled, vitest finishes immediately with an error.
Yes I thought it is better to use AggregateError for DX.

I think we might need a better error message showing where errors came from.

I thought it will be better to have those too but I didn't come up with how to implement it.
Currently, I don't understand why the error is coming from pool.run.

fix: handle error from pool.run

d28cbf0

sheremet-va requested changes May 21, 2022

View reviewed changes

packages/vitest/src/node/pool.ts Outdated Show resolved Hide resolved

packages/vitest/src/utils/index.ts Outdated Show resolved Hide resolved

chore: lint fix

a23fb58

chore: update aggregate error message

0ab3996

chore: use AggregateErrorPonyfill consistently

7478246

sheremet-va approved these changes May 21, 2022

View reviewed changes

sapphi-red mentioned this pull request May 21, 2022

chore: make terser an optional dependency vitejs/vite#8049

Merged

9 tasks

antfu approved these changes May 21, 2022

View reviewed changes

antfu merged commit fbaa546 into vitest-dev:main May 21, 2022

sapphi-red deleted the fix/handle-error-pool-run branch May 22, 2022 07:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: handle error from pool.run #1348

fix: handle error from pool.run #1348

sapphi-red commented May 21, 2022

netlify bot commented May 21, 2022 •

edited

Loading

sheremet-va left a comment

sheremet-va commented May 21, 2022

sapphi-red commented May 21, 2022

sheremet-va commented May 21, 2022

sapphi-red commented May 21, 2022 •

edited

Loading

fix: handle error from pool.run #1348

fix: handle error from pool.run #1348

Conversation

sapphi-red commented May 21, 2022

Before

After

netlify bot commented May 21, 2022 • edited Loading

✅ Deploy Preview for vitest-dev ready!

sheremet-va left a comment

Choose a reason for hiding this comment

sheremet-va commented May 21, 2022

sapphi-red commented May 21, 2022

sheremet-va commented May 21, 2022

sapphi-red commented May 21, 2022 • edited Loading

netlify bot commented May 21, 2022 •

edited

Loading

sapphi-red commented May 21, 2022 •

edited

Loading