Defer resolving globalThis.fetch until calling endpoint

[paul-sachs]

When using a polyfilled (or patched) fetch, we need to make sure the fetch is retrieved at call site, rather than being cached earlier. This allows tools like Sentry to properly detect fetch calls.

[timostamm]

Good call, but left a couple of comments. You have to run make bench to update the code size benchmark too.

[paul-sachs]

You have to run make bench to update the code size benchmark too.

I ran make bench and updated the readme but CI continues to fail due to a change in the size. Is there a specific version of node I need to run it with?

[timostamm]

For manipulating globalThis.fetch in the tests, it looks like it would be useful to use a setup and teardown, so that a test failure doesn't affect all other tests:

    let originFetch: typeof fetch;
    beforeEach(() => (originFetch = globalThis.fetch));
    afterEach(() => (globalThis.fetch = originFetch));

For the gRPC-web test, mocking the response will be difficult, because the transport uses the body as a stream.

Since we do not care which parts of the response are used in this test, you can reject from your fetch instead of resolving:

const transport = createGrpcWebTransport({
  baseUrl: "https://example.com",
});
// Patch globalThis.fetch to mimic a polyfill or patch
globalThis.fetch = () => Promise.reject("test-error-raised-from-patched-fetch");
await expectAsync(
  transport.unary(
    TestService,
    TestService.methods.unaryCall,
    undefined,
    undefined,
    undefined,
    new SimpleRequest()
  )
).toBeRejectedWithError(/test-error-raised-from-patched-fetch/);

[paul-sachs]

@timostamm Yeah, that makes a lot of sense. I was seeing that weirdness and just started to mess with beforeEach/afterEach but was looking into the GRPC thing first. I was kinda struggling to mock it out so the error idea is super helpful, thanks!