Bump undici #510
Bump undici #510
Conversation
|
|
Hey! 👋 Thanks for the PR @WalshyDev and @Cherry. I've pushed some fixes that are passing tests locally, let's see if they pass CI. 🙂 I've reverted the bump to Node |
| @@ -2,6 +2,8 @@ declare global { | |||
| // TODO(soon): remove once included in `@types/node` | |||
| // eslint-disable-next-line no-var | |||
| var CryptoKey: typeof import("crypto").webcrypto.CryptoKey; | |||
| // eslint-disable-next-line no-var | |||
| var MessagePort: typeof import("worker_threads").MessagePort; | |||
There was a problem hiding this comment.
This was required for undici type checking to pass.
| globalThis.ReadableStream = stream.ReadableStream; | ||
| globalThis.WritableStream = stream.WritableStream; | ||
| globalThis.TransformStream = stream.TransformStream; | ||
| require("undici/lib/fetch"); |
There was a problem hiding this comment.
Just importing undici doesn't work here, as the fetch implementation is lazily imported
packages/core/src/standards/http.ts
Outdated
| export class Request extends Body<BaseRequest> { | ||
| // noinspection TypeScriptFieldCanBeMadeReadonly | ||
| #cf?: IncomingRequestCfProperties | RequestInitCfProperties; | ||
|
|
||
| constructor(input: RequestInfo, init?: RequestInit) { | ||
| // If body is a FixedLengthStream, set Content-Length to its expected length | ||
| const contentLength: number | undefined = (init?.body as any)?.[ |
There was a problem hiding this comment.
We may replace init.body later on with a different ReadableStream, so extract contentLength first
| async pull(controller) { | ||
| let result = await reader.read(); | ||
| while (!result.done && result.value.byteLength === 0) { | ||
| result = await reader.read(); | ||
| } | ||
| if (result.done) { | ||
| queueMicrotask(() => controller.close()); | ||
| } else if (result.value.byteLength > 0) { | ||
| controller.enqueue(result.value); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Should this be reading the whole stream in one invocation of pull?
There was a problem hiding this comment.
It will only read the whole stream if all chunks are empty, which seems unlikely. The while loop is just there to ensure we get a non-empty result to enqueue.
This applies the same fix for `Request` bodies from #510 to `Response`s. Notably, byte streams are returned from lots of Workers runtime APIs (e.g. KV, R2) to support BYOB reads. It's likely these are then used in `Response`s and cloned for caching.
This applies the same fix for `Request` bodies from #510 to `Response`s. Notably, byte streams are returned from lots of Workers runtime APIs (e.g. KV, R2) to support BYOB reads. It's likely these are then used in `Response`s and cloned for caching.
This PR bumps undici to 5.20.0 which resolves GHSA-5r9g-qh6m-jxff and GHSA-r6ch-mqf9-qc9w. This removes the scary npm audit warnings when installing wrangler.
Continuing #508
Few things to note:
Request#duplexnow required ifbodyis provided inRequestInitStill working through the few tests still failing... not sure why they're failing yet though.
For some reason it's detecting ReadableStream in use even for a very basic test: