-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Fix/web stream error handling #14732
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
base: main
Are you sure you want to change the base?
Changes from all commits
62bac64
c9592a3
d74c401
462048b
53ee778
403ce6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { createApp, defineAsyncComponent, h } from 'vue' | ||
| import { pipeToNodeWritable, renderToNodeStream } from '../src' | ||
| import { Writable } from 'node:stream' | ||
| import { describe, expect, test } from 'vitest' | ||
|
|
||
| describe('Node.js Streams backpressure', () => { | ||
| test('pipeToNodeWritable backpressure', async () => { | ||
| const Async = defineAsyncComponent(() => | ||
| Promise.resolve({ | ||
| render: () => h('div', 'b'), | ||
| }), | ||
| ) | ||
| const App = { | ||
| render: () => [h('div', 'a'), h(Async)], | ||
| } | ||
|
|
||
| let writeCount = 0 | ||
| const writable = new Writable({ | ||
| highWaterMark: 1, | ||
| write(_chunk, _encoding, callback) { | ||
| writeCount++ | ||
| callback() | ||
| }, | ||
| }) | ||
|
|
||
| const originalWrite = writable.write.bind(writable) | ||
| let firstCall = true | ||
| writable.write = (chunk: any, encoding?: any, cb?: any): any => { | ||
| if (firstCall) { | ||
| firstCall = false | ||
| originalWrite(chunk, encoding, cb) | ||
| return false | ||
| } | ||
| return originalWrite(chunk, encoding, cb) | ||
| } | ||
|
|
||
| pipeToNodeWritable(createApp(App), {}, writable) | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 20)) | ||
| // Should have only 1 write because it returned false and we're waiting for drain | ||
| expect(writeCount).toBe(1) | ||
|
|
||
| writable.emit('drain') | ||
| await new Promise(resolve => setTimeout(resolve, 20)) | ||
| // Second write should have happened after drain | ||
| expect(writeCount).toBeGreaterThan(1) | ||
| }) | ||
|
|
||
| test('renderToNodeStream backpressure', async () => { | ||
| const Async = defineAsyncComponent(() => | ||
| Promise.resolve({ | ||
| render: () => h('div', 'b'), | ||
| }), | ||
| ) | ||
| const App = { | ||
| render: () => [h('div', 'a'), h(Async)], | ||
| } | ||
|
|
||
| const stream = renderToNodeStream(createApp(App)) | ||
|
|
||
| // In Node.js Readable, push() returns false when the buffer is full. | ||
| // For our test, we'll just verify that it streams correctly first. | ||
| let res = '' | ||
| for await (const chunk of stream) { | ||
| res += chunk | ||
| } | ||
| expect(res).toBe('<!--[--><div>a</div><div>b</div><!--]-->') | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -64,3 +64,85 @@ test('pipeToWebWritable', async () => { | |||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| expect(res).toBe(`<!--[--><div>parent</div><div>async</div><!--]-->`) | ||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| test('pipeToWebWritable error handling', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const App = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| ssrRender() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('ssr render error') | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| let abortedReason: any | ||||||||||||||||||||||||||||||||||||||||||||||||
| const writable = new WritableStream({ | ||||||||||||||||||||||||||||||||||||||||||||||||
| abort(reason) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| abortedReason = reason | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| pipeToWebWritable(createApp(App), {}, writable) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Wait for the error to propagate | ||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 10)) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| expect(abortedReason).toBeInstanceOf(Error) | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(abortedReason.message).toBe('ssr render error') | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+82
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flaky wait — prefer event-driven synchronization over a fixed 10 ms timeout.
🧪 Proposed fix let abortedReason: any
+ let resolveAbort!: () => void
+ const aborted = new Promise<void>(r => (resolveAbort = r))
const writable = new WritableStream({
abort(reason) {
abortedReason = reason
+ resolveAbort()
},
})
pipeToWebWritable(createApp(App), {}, writable)
- // Wait for the error to propagate
- await new Promise(resolve => setTimeout(resolve, 10))
+ await aborted
expect(abortedReason).toBeInstanceOf(Error)
expect(abortedReason.message).toBe('ssr render error')📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| test('pipeToWebWritable backpressure', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const Async = defineAsyncComponent(() => | ||||||||||||||||||||||||||||||||||||||||||||||||
| Promise.resolve({ | ||||||||||||||||||||||||||||||||||||||||||||||||
| render: () => h('div', 'b'), | ||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| const App = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| render: () => [h('div', 'a'), h(Async)], | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| let writeCount = 0 | ||||||||||||||||||||||||||||||||||||||||||||||||
| let resolveWrite: any | ||||||||||||||||||||||||||||||||||||||||||||||||
| const writable = new WritableStream({ | ||||||||||||||||||||||||||||||||||||||||||||||||
| write() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| writeCount++ | ||||||||||||||||||||||||||||||||||||||||||||||||
| return new Promise(resolve => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| resolveWrite = resolve | ||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| pipeToWebWritable(createApp(App), {}, writable) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 20)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Should have only 1 write because the first one is pending | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(writeCount).toBe(1) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| resolveWrite() | ||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 20)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Second write should have happened after the async component resolved | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(writeCount).toBeGreaterThan(1) | ||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| test('renderToWebStream backpressure', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const Async = defineAsyncComponent(() => | ||||||||||||||||||||||||||||||||||||||||||||||||
| Promise.resolve({ | ||||||||||||||||||||||||||||||||||||||||||||||||
| render: () => h('div', 'b'), | ||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| const App = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| render: () => [h('div', 'a'), h(Async)], | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const stream = renderToWebStream(createApp(App), {}) | ||||||||||||||||||||||||||||||||||||||||||||||||
| const reader = stream.getReader() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const { value: v1 } = await reader.read() | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(new TextDecoder().decode(v1)).toBe('<!--[--><div>a</div>') | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const { value: v2 } = await reader.read() | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(new TextDecoder().decode(v2)).toBe('<div>b</div>') | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const { value: v3 } = await reader.read() | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(new TextDecoder().decode(v3)).toBe('<!--]-->') | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const { done } = await reader.read() | ||||||||||||||||||||||||||||||||||||||||||||||||
| expect(done).toBe(true) | ||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the backpressure test control
draindeterministically.Calling the original
Writable.write()and immediately invoking its callback lets Node emit a realdrainbefore Line 43, so the Line 41 assertion can race and fail before the manualdrain.Proposed deterministic gate
🤖 Prompt for AI Agents