Skip to content

fix: add handshake timeout to iframe communication#10656

Merged
hi-ogawa merged 5 commits into
mainfrom
fix/timeout-iframe-message
Jun 26, 2026
Merged

fix: add handshake timeout to iframe communication#10656
hi-ogawa merged 5 commits into
mainfrom
fix/timeout-iframe-message

Conversation

@sheremet-va

@sheremet-va sheremet-va commented Jun 24, 2026

Copy link
Copy Markdown
Member

This PR adds a handshake timeout to catch iframes that were not setup properly. This should help catch bugs - normally, the timeout is not required.

@netlify

netlify Bot commented Jun 24, 2026

Copy link
Copy Markdown

Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit 0686d56
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/6a3b9c5fc9127a0008c0085c
😎 Deploy Preview https://deploy-preview-10656--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

@sheremet-va
sheremet-va requested a review from hi-ogawa June 24, 2026 11:33
@sheremet-va
sheremet-va marked this pull request as ready for review June 24, 2026 11:33
Comment thread test/browser/specs/readiness.test.ts Outdated
},
)

expect(stderr).toContain('did not become ready within 2000ms')

@hi-ogawa hi-ogawa Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Stderr looks narrow to show in which layer the error is handled. Can you add error tree so it's easy to tell if this becomes file entity error or unhandled error?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added

@hi-ogawa hi-ogawa Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The new assertion didn't clarified for me much. I just dumped locally to get the idea.
Error tree will include __unhandled_errors__ but doesn't show cause so direct stderr assertion seems fine.

Case 2 — "fails instead of hanging when the tester never becomes ready"
⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯
Error: Failed to run the test /Users/hogawa/code/others/vitest-pr-10656/test/browser/vitest-test-11c97409-1b0f-4d9c-aa16-a9d7ff519bda/basic.test.ts.
 ❯ ../../../packages/vitest/dist/chunks/cli-api.CIsh518E.js:4957:17
 ❯ processTicksAndRejections ../node:internal/process/task_queues:104:5

Caused by: Error: The iframe "/Users/hogawa/code/others/vitest-pr-10656/test/browser/vitest-test-11c97409-1b0f-4d9c-aa16-a9d7ff519bda/basic.test.ts" did not become ready within 2000ms. The tester likely failed to initialize, check the browser console for errors.
 ❯ ../../../../../../../../__vitest_browser__/orchestrator-pKob2Z-i.js:188:28

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Case 3 — "fails instead of hanging when the tester stops responding to messages"
⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯
Error: Failed to run the test /Users/hogawa/code/others/vitest-pr-10656/test/browser/vitest-test-c6ba9b4e-3d1d-431a-86af-96d692eb11f2/basic.test.ts.
 ❯ ../../../packages/vitest/dist/chunks/cli-api.CIsh518E.js:4957:17
 ❯ processTicksAndRejections ../node:internal/process/task_queues:104:5

Caused by: Error: The iframe "/Users/hogawa/code/others/vitest-pr-10656/test/browser/vitest-test-c6ba9b4e-3d1d-431a-86af-96d692eb11f2/basic.test.ts" did not acknowledge the "prepare" message within 2000ms. The tester might have crashed, been removed, or be blocked by a long synchronous task.
 ❯ ../../../../../../../../__vitest_browser__/orchestrator-pKob2Z-i.js:332:28

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

@hi-ogawa hi-ogawa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good to me except one test nit.

@hi-ogawa
hi-ogawa merged commit 3545fe7 into main Jun 26, 2026
15 of 18 checks passed
@hi-ogawa
hi-ogawa deleted the fix/timeout-iframe-message branch June 26, 2026 00:39
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.

2 participants