orval
orval copied to clipboard
204 No content generated code throws JSON parsing error
What are the steps to reproduce this issue?
- Create new project with
fetchclient from openapi scheme which returns 204 (No content). - Try calling the generated method.
- You should get error Unexpected end of JSON input error.
What happens?
Basically when there is no response in fetch client response there is no content. Which mean the res.json() will throw error.
Generated code looks like this:
const data = await res.json()
return { status: res.status, data }
What were you expecting to happen?
There shouldn't be a error.
Any logs, error output, etc?
SyntaxError: Unexpected end of JSON input
at JSON.parse (<anonymous>)
at parseJSONFromBytes (node:internal/deps/undici/undici:4306:19)
at successSteps (node:internal/deps/undici/undici:4288:27)
at consumeBody (node:internal/deps/undici/undici:4294:9)
at _Response.json (node:internal/deps/undici/undici:4239:18)
Any other comments?
I think when there is 204 in specs or there is no content in openapi specification the const data should not be generated and data should not be required in generated responses as there are none.
What versions are you using?
System:
OS: Linux 6.9 Pop!_OS 22.04 LTS
CPU: (24) x64 AMD Ryzen 9 7900X 12-Core Processor
Memory: 9.64 GB / 31.08 GB
Container: Yes
Shell: 5.1.16 - /bin/bash
npmPackages:
orval: ^7.1.1 => 7.1.1
react: rc => 19.0.0-rc-0751fac7-20241002
zod: 3.23.8 => 3.23.8
@zZHorizonZz
There is no solution for this yet, but you can work-around it by using a custom fetch like below:
const getBody = async <T>(c: Response | Request): Promise<T> => {
const text = await c.text();
if (!text) {
return {} as T;
}
const contentType = c.headers.get("content-type");
if (contentType?.includes("application/json")) {
return JSON.parse(text);
}
return text as unknown as T;
};
export const customFetch = async <T>(
url: string,
options: RequestInit,
): Promise<T> => {
const response = await fetch();
const data = await getBody<T>(response);
return { status: response.status, data } as T;
};
Hello,
I'm facing the same issue. My opennapi.json says 204 (No Content), but orval doesn't take this into account when using fetch (and it incorrectly calls a .json()), the generated code looks like this:
export const getDeleteAllDoneTasksUrl = () => {
return `/api/tasks/done`
}
export const deleteAllDoneTasks = async ( options?: RequestInit): Promise<deleteAllDoneTasksResponse> => {
const res = await fetch(getDeleteAllDoneTasksUrl(),
{
...options,
method: 'DELETE'
}
)
// BUG: Throws an exception if the response contains no content (body):
const data = waiting for res.json()
return { status: res.status, data }
}
I tried the fix from https://github.com/orval-labs/orval/issues/1656#issuecomment-2408768709 as it is and registered it in orval.config.js:
..
override: {
mutator: {
path: './custom-fetch.ts',
name: 'customFetch',
},
..
However, it doesn't work for me as it breaks other APIs (which require a json()-call) - I don't know if this is related to https://github.com/orval-labs/orval/pull/1632.
@nimo23
Among the automatically generated custom hooks, custom fetch is called. Are your settings correct?
https://orval.dev/guides/custom-client
Among the automatically generated custom hooks, custom fetch is called. Are your settings correct?
Yes, the custom fetch will be called.
@nimo23 If configured correctly, it can be customized. Where exactly does the error occur?
@soartec-lab thanks, With the help of https://github.com/orval-labs/orval/blob/master/samples/next-app-with-fetch/custom-fetch.ts I get it working.
@nimo23 it's good. I'll close this issue and reopen it if you need it again.
@soartec-lab Since orval should aim to adhere to the definitions set in openapi.yaml, it should fix this issue automatically (without requiring user intervention). I can imagine that others will also have this problem in the future.
The simple fact is that something like 204 No Content doesn't have a content response per definition, so by default Orval shouldn't try to retrieve a content (via json()). Because this inevitably leads to errors. It would be better to fix this bug. Don't you think so?
@nimo23
I agree, I think it's good to support not content.
However, there are many things to consider before and after `fetch', and we wanted to make the automatically generated functions as simple as possible.
If we can accommodate this while maintaining simplicity, I'm all for it.
The main problem is that attempting to call .json() throws an error when there is no content in the response. Instead of throwing an error, I think the generated code should handle this gracefully, when there is no content in the response (whether it's 204 No Content or any other response without a body), the method should return {status: statusCode} without attempting to parse the non-existent body. What do you think @soartec-lab ?
Something like
try{
const data = await res.json()
}
catch(Error error){
// no content
}
or using something like
// in future: ECMAScript Safe Assignment Operator (?=)
const data ?= await res.json()
OK, I'll think about this, so give me some time and this issue be reopened.
I have a couple ideas:
1. Check res.body
In this case, you can simply remove the error since it doesn't depend on the status code.
export const listPets = async (
params?: ListPetsParams,
options?: RequestInit,
): Promise<listPetsResponse> => {
const res = await fetch(getListPetsUrl(params), {
...options,
method: 'GET',
});
- const data: Pets = await res.json();
+ const data: Pets = res.body ? await res.json() : {};
return { status: res.status, data, headers: res.headers };
};
2. Check the status code
In this case, you will more strictly follow the REST API specifications. For example, if there is no response even though it is 200, you will still throw an exception.
export const listPets = async (
params?: ListPetsParams,
options?: RequestInit,
): Promise<listPetsResponse> => {
const res = await fetch(getListPetsUrl(params), {
...options,
method: 'GET',
});
- const data: Pets = await res.json();
+ const data: Pets = [204, 304].includes(res.status) ? await res.json() : {};
return { status: res.status, data, headers: res.headers };
};
We will be looking into this further, but do you have any opinions so far?
To me personally, it makes sense to only use option with the No Content (204, 304) response when there's actually, well, no content. But APIs aren't always perfect, and I've seen some that return a 200 status with no content. So, I think the first option would be better because it would cover those weird APIs too.
Thank you for your reply. I agree with your opinion. I will wait for more options to gather.
I think the 5 possible states should be handled like this:
- Present unrequired body (ignore)
- Missing required body (accept anyway)
- Present but invalid JSON body (error)
- Present optional/required body (expected)
- Missing optional/unrequired body (expected)
The following status codes should not have bodies according to the spec: 1xx, 204, 205, 304. When these codes are used, the body should always be ignored, even if present (state 1).
As @zZHorizonZz noted, some APIs omit bodies even when they're technically required. This should be accepted (state 2).
@nimo23 mentions in https://github.com/orval-labs/orval/pull/1778#discussion_r1899133450 that with the current suggestion, a present but empty body will fail. I think that is correct behavior, since it is invalid JSON (state 3).
States 4 and 5 represent the expected behavior. I therefore propose handling them explicitly, while treating the others as mentioned:
export const listPets = async (
params?: ListPetsParams,
options?: RequestInit,
): Promise<listPetsResponse> => {
const res = await fetch(getListPetsUrl(params), {
...options,
method: 'GET',
});
- const data: Pets = await res.json();
+ const data: Pets = (res.body && ![204, 205, 304].includes(res.status))
+ ? await res.json()
+ : {};
return { status: res.status, data, headers: res.headers };
};
Excellent summary @clemeth
Thank you for putting this together. I understand that you have merged both of my ideas. I completely agree with this, so I would like to implement this.