When a server returns an error status code without content, the error is present but undefined

[julienw]

openapi-fetch version

Latest

Description

The context is this code:

openapi-typescript/packages/openapi-fetch/src/index.js

Line 235 in 12f9c29

return response.ok ? { data: undefined, response } : { error: undefined, response };

I wonder if we shouldn't have a tru-ish value (maybe just "true") for error here, so that the calling code can simply use if (error). What do you think?

Reproduction

The problem happens when a server returns an error status (such as 405) without any content.

Expected result

The caller can do if (error) simply.

Extra

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

[matteom-synth]

I've also just been bitten by this default behaviour, which I feel is quite dangerous. I would also recommend returning some sort of truthy value here, but I can see it being problematic with the current way the error is returned.
I understand it would be a breaking change (and I'm probably missing context!) but I feel it might be easier if the error was an object with "status" and "content" (or something similar) so that consumers could always have access to these informations while keeping the option of having an empty response.

[matteom-synth]

To avoid patching the lib, for now I'm using a middleware to handle this case (still haven't tested it properly):

function emptyErrorResponseMiddleware(): Middleware {
  return {
    async onResponse({ response }) {
      if (response.ok) {
        return response;
      }
      const error = await response.clone().text();
      if (error) {
        return response;
      }
      return new Response("Empty response", response);
    },
  };
}

[julienw]

I guess we could always check response.ok instead of the returned error property.
or test something such as if ("error" in returnValue) instead of destructuring.

But this is very misleading :/

[matteom-synth]

FYI @julienw I noticed you linked a piece of code mentioning it being responsible for this, but the problem is actually happening here (for me at least), since error is empty in this context.

[julienw]

Interesting! In my case I had Content-Length: 0 so I was in the first case. In your case then error should be defined, but maybe it's the empty string (which is falsy)?

[matteom-synth]

Ah, I see. Yeah then both checks are problematic. Content-Length: 0 first (but Content-Length might not be sent from the server) and then the falsy error (empty string).

[emilhaugberg]

We have been bitten by this logic too. We use openapi-react-query (uses openapi-fetch under the hood), which wouldn't trigger onError due to an empty error response body. We ended up patching it directly in the openapi-react-query implementation:

     const { data, error, response } = await fn(path, { signal, ...init });
-    if (error) {
-      throw error;
+    if (!response.ok) {
+      throw error ? error : new Error("Unkown error occurred.");

This is not ideal. Will this be looked at at some point?