onRequest: must return new Request() when modifying the request

[WesleyYue]

Description

I get the following error occasionally in a api client with a simple auth middleware:
onRequest: must return new Request() when modifying the request

The error seems to be triggered because my middleware was handed a request that fails the check here, which my code then returns from the middleware and triggers the error.

It's not clear to me what triggers this. It only happens intermittently that I am able to capture in the wild. My environment is a VSCode extension host, which is essentially a nodejs environment in electron.

Any ideas how I can further debug this?

Reproduction

export class ApiManager {
  public api: ReturnType<typeof createClient<paths>>;
  constructor(private authManager: AuthManager) {
    this.api = createClient<paths>({
      baseUrl: new URL("api", process.env.VITE_API_SERVER_URL).toString(),
    });
    this.api.use(this.authMiddleware());
  }

  private authMiddleware(): Middleware {
    return {
      onRequest: ({ request }) => {
        const accessToken = this.authManager.accessToken;

        if (!accessToken) {
          throw new Error("Failed to retrieve access token.");
        }

        request.headers.set("Authorization", `Bearer ${accessToken}`);

        try {
          // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
          if (request && !(request instanceof Request)) {
            logger.warn("Got a non-request request in middleware.", { request });
            scope.captureException(new Error("Got a non-request request in middleware."), {
              data: { request },
            });
          }
        } catch (e) {
          // Don't fail on this.
          logger.error("Error in authMiddleware", { error: e });
        }
        return request;
      },
    };
  }
}

Expected result

Middleware should receive a request that is actually a Request.
It should not be handed a object that will fail the subsequent Request check without any changes.

Checklist

• I’m willing to open a PR (see CONTRIBUTING.md)

[ItsJustRuby]

Hi @WesleyYue,

sorry for digging up this issue - but I recently encountered the same behavior, and wanted to share the solution to your issue:

The API here is such that a middleware must either

• "modify" the request context without modifying the actual request variable by instead returning a new request object instantiated using the new Request() constructor
• or if not wanting to make any changes to the request context, return undefined instead (in which case the check on line 117 will skip the check on line 118 instead)

[drwpow]

Yeah this is tricky. The “returning a new Request” restriction doesn’t come from this library, but the fetch API itself in all browsers. We just throw an error that makes it clear that it happens in openapi-fetch, but if that weren’t there, you’d see an error thrown by the client itself that would be a little more obtuse.

Looking that that check, that did get touched when we added the ability for someone to add custom properties to Request (which was needed for Next.js and some Node setups):

if (!(result instanceof CustomRequest)) {
  throw new Error("onRequest: must return new Request() when modifying the request");
}

Originally that used to be instanceof Request, and I think that would still work and cause less disruption. I’d accept a PR for that change, and see if that reduces the errors you’re seeing (and if I’m remembering my animals correctly, would be more correct). Alternatively, we could just remove that check altogether and just let the client throw the error (which I’m not opposed to—it could have more helpful information, perhaps)

I don’t see anything in your code example that smells like anything is wrong on your end; this seems like that error condition is wrong.