Try to fix flakey socket hang up failures in stream cancel tests (#53318)
### What?
I think the flakiness from the new stream cancel tests is due to a uncaught error thrown from the priming. If it is during the priming, then we need to also explicitly check if we restart the test and reset the `streamable`.
### Why?
Flakey tests suck.
### How?
- Adds a `on('error', reject)` to catch the socket error and associate it with the test
- Explicitly checks for the `?write=` param to see if we're priming or getting results
This commit is contained in:
Justin Ridgewell
GitHub
parent
ce29de25e4
commit
3906374740
6 changed files with 76 additions and 77 deletions
|
|
@ -9,19 +9,19 @@ export async function GET(req: Request): Promise<Response> {
|
|||
// This is so we don't confuse the request close with the connection close.
|
||||
await req.text()
|
||||
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
if (streamable) {
|
||||
const old = streamable
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
if (write) {
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
}
|
||||
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
const old = streamable!
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,19 +11,19 @@ export async function GET(req: Request): Promise<Response> {
|
|||
// This is so we don't confuse the request close with the connection close.
|
||||
await req.text()
|
||||
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
if (streamable) {
|
||||
const old = streamable
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
if (write) {
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
}
|
||||
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
const old = streamable!
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,19 +11,20 @@ export default async function handler(req: Request): Promise<Response> {
|
|||
// This is so we don't confuse the request close with the connection close.
|
||||
await req.text()
|
||||
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
if (streamable) {
|
||||
const old = streamable
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
|
||||
if (write) {
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
}
|
||||
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
const old = streamable!
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,19 +11,19 @@ export default async function handler(req: Request): Promise<Response> {
|
|||
// This is so we don't confuse the request close with the connection close.
|
||||
await req.text()
|
||||
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
if (streamable) {
|
||||
const old = streamable
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
if (write) {
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
}
|
||||
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
const s = (streamable = Streamable(+write!))
|
||||
req.signal.onabort = () => {
|
||||
s.abort()
|
||||
}
|
||||
return new Response(s.stream)
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
const old = streamable!
|
||||
streamable = undefined
|
||||
const i = await old.finished
|
||||
return new Response(`${i}`)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,25 +15,25 @@ export default function handler(
|
|||
// Pages API requests have already consumed the body.
|
||||
// This is so we don't confuse the request close with the connection close.
|
||||
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
// The 2nd request should render the stats. We don't use a query param
|
||||
// because edge rendering will create a different bundle for that.
|
||||
if (readable) {
|
||||
const old = readable
|
||||
readable = undefined
|
||||
return old.finished.then((i) => {
|
||||
res.end(`${i}`)
|
||||
if (write) {
|
||||
const r = (readable = Readable(+write!))
|
||||
res.on('close', () => {
|
||||
r.abort()
|
||||
})
|
||||
return new Promise((resolve) => {
|
||||
pipeline(r.stream, res, () => {
|
||||
resolve()
|
||||
res.end()
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
const write = new URL(req.url!, 'http://localhost/').searchParams.get('write')
|
||||
const r = (readable = Readable(+write!))
|
||||
res.on('close', () => {
|
||||
r.abort()
|
||||
})
|
||||
return new Promise((resolve) => {
|
||||
pipeline(r.stream, res, () => {
|
||||
resolve()
|
||||
res.end()
|
||||
})
|
||||
const old = readable!
|
||||
readable = undefined
|
||||
return old.finished.then((i) => {
|
||||
res.end(`${i}`)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ createNextDescribe(
|
|||
jest.retryTimes(3)
|
||||
|
||||
function prime(url: string, noData?: boolean) {
|
||||
return new Promise<void>((resolve) => {
|
||||
return new Promise<void>((resolve, reject) => {
|
||||
url = new URL(url, next.url).href
|
||||
|
||||
// There's a bug in node-fetch v2 where aborting the fetch will never abort
|
||||
|
|
@ -29,6 +29,7 @@ createNextDescribe(
|
|||
res.destroy()
|
||||
resolve()
|
||||
})
|
||||
req.on('error', reject)
|
||||
req.end()
|
||||
|
||||
if (noData) {
|
||||
|
|
@ -58,9 +59,8 @@ createNextDescribe(
|
|||
it('cancels stream making progress', async () => {
|
||||
// If the stream is making regular progress, then we'll eventually hit
|
||||
// the break because `res.destroyed` is true.
|
||||
const url = path + '?write=25'
|
||||
await prime(url)
|
||||
const res = await next.fetch(url)
|
||||
await prime(path + '?write=25')
|
||||
const res = await next.fetch(path)
|
||||
const i = +(await res.text())
|
||||
expect(i).toBeWithin(1, 5)
|
||||
}, 2500)
|
||||
|
|
@ -68,9 +68,8 @@ createNextDescribe(
|
|||
it('cancels stalled stream', async () => {
|
||||
// If the stream is stalled, we'll never hit the `res.destroyed` break
|
||||
// point, so this ensures we handle it with an out-of-band cancellation.
|
||||
const url = path + '?write=1'
|
||||
await prime(url)
|
||||
const res = await next.fetch(url)
|
||||
await prime(path + '?write=1')
|
||||
const res = await next.fetch(path)
|
||||
const i = +(await res.text())
|
||||
expect(i).toBe(1)
|
||||
}, 2500)
|
||||
|
|
@ -78,9 +77,8 @@ createNextDescribe(
|
|||
it('cancels stream that never sent data', async () => {
|
||||
// If the client has never sent any data (including headers), then we
|
||||
// haven't even established the response object yet.
|
||||
const url = path + '?write=0'
|
||||
await prime(url, true)
|
||||
const res = await next.fetch(url)
|
||||
await prime(path + '?write=0', true)
|
||||
const res = await next.fetch(path)
|
||||
const i = +(await res.text())
|
||||
expect(i).toBe(0)
|
||||
}, 2500)
|
||||
|
|
|
|||
Loading…
Reference in a new issue