Fix null status body response

[GregBrimble]

import { fetch } from '@miniflare/core'

await fetch('https://some/url/which/returns/304')

fails because response.body always returns a ReadableStream, but the new Response() constructor expects null when setting some status codes.

[mrbbot]

Looks good! This will close #165. Do you have time to write a test too: https://github.com/cloudflare/miniflare/blob/master/packages/core/test/standards/http.spec.ts? 🙂

[mrbbot]

Nit: nullBodyStatus is defined earlier on in this file. This doesn't include 101, that should probably be added.

Suggested change

	[101, 204, 205, 304].includes(baseRes.status) ? null : baseRes.body,
	nullBodyStatus.includes(baseRes.status) ? null : baseRes.body,

[alexandrukis]

Disclaimer: I am very new to miniflare, so feel free to ignore my comment.

Nitpick: there are so many things happening in this function.
This is already an improvement in readability IMO:

const body = [101, 204, 205, 304].includes(baseRes.status) ? null : baseRes.body;
res = new Response(body, baseRes);

but the whole if/else could be moved to a separate function that would simply return the new response

function toHybridResponse(baseRes: Response): Response { // or any other naming that fits the project conventions
  if (baseRes.type !== "opaqueredirect") {
    const body = [101, 204, 205, 304].includes(baseRes.status) ? null : baseRes.body;
    return new Response(body, baseRes);
  }

  // Unpack opaque responses. This restriction isn't needed server-side,
  // and Cloudflare doesn't support Response types anyway.
  // @ts-expect-error symbol properties are not included in type definitions
  const internalResponse = baseRes[fetchSymbols.kState].internalResponse;
  const headersList = internalResponse.headersList;
  assert(headersList.length % 2 === 0);
  const headers = new Headers();
  for (let i = 0; i < headersList.length; i += 2) {
    headers.append(headersList[i], headersList[i + 1]);
  }
  // Cloudflare returns a body here, but undici aborts the stream so
  // unfortunately it's unusable :(
  return new Response(null, {
    status: internalResponse.status,
    statusText: internalResponse.statusText,
    headers,
  });
}

[petebacondarwin]

LGTM!

[mrbbot]

Thank you! 😃

[mrbbot]

Hey! 👋 I've just released version 2.3.0 including this PR. You can find the full changelog here.