Skip to content

Commit 31661f3

Browse files
Reflexclaude
andcommitted
fix(h2-transport): address review nits (unreachable 101, url descriptor, stale comment)
Per review on #815: - Drop `101` from NULL_BODY_STATUSES — Switching Protocols is an HTTP/1.1 upgrade status and can't occur over HTTP/2, so it was unreachable. - Add `writable: true` to the `Response.url` property descriptor. - Fix a stale comment in tests/index.test.ts that referenced the pre-refactor `useHttp2` / `makeHttp2Fetch(...)` shape; it's now `resolveHttp2Fetch(options)`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f37c3c5 commit 31661f3

2 files changed

Lines changed: 5 additions & 4 deletions

File tree

src/lib/h2-transport/index.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ import { Response } from 'node-fetch';
2323
import { type Fetch } from '../../core';
2424

2525
// Statuses that must not carry a body per the Fetch spec; node-fetch's Response
26-
// rejects a non-null body for these.
27-
const NULL_BODY_STATUSES = new Set([101, 204, 205, 304]);
26+
// rejects a non-null body for these. (101 is an HTTP/1.1 upgrade status and can't
27+
// occur over HTTP/2, so it's omitted.)
28+
const NULL_BODY_STATUSES = new Set([204, 205, 304]);
2829

2930
const MIN_NODE_MAJOR = 18;
3031

@@ -84,7 +85,7 @@ function toFetchResponse(h2: H2Response): Response {
8485

8586
// node-fetch derives `url` from the request internals it never saw; expose the
8687
// real request URL (`Response.url` is a prototype getter, shadowed here).
87-
Object.defineProperty(response, 'url', { value: h2.url, configurable: true });
88+
Object.defineProperty(response, 'url', { value: h2.url, writable: true, configurable: true });
8889
return response;
8990
}
9091

tests/index.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ describe('instantiate client', () => {
100100
test('custom fetch wins over http2', async () => {
101101
// When both `fetch` and `http2` are provided, the custom fetch must be used —
102102
// the h2 adapter should not run. Locks in src/index.ts:
103-
// fetch: options.fetch ?? (useHttp2 ? makeHttp2Fetch(...) : undefined)
103+
// fetch: options.fetch ?? resolveHttp2Fetch(options)
104104
const customFetch = jest.fn((url: RequestInfo) =>
105105
Promise.resolve(
106106
new Response(JSON.stringify({ url, custom: true }), {

0 commit comments

Comments
 (0)