Skip to content

Commit 4a36aa0

Browse files
Merge pull request #569 from effector/fix-no-content-response
Fix "no content" response support
2 parents d63aecc + 8ba7008 commit 4a36aa0

6 files changed

Lines changed: 138 additions & 3 deletions

File tree

.changeset/beige-teams-bet.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@farfetched/core': patch
3+
---
4+
5+
Fix "No Content" responses support

packages/core/src/fetch/__tests__/json.response.data.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,52 @@ describe('fetch/json.response.data', () => {
106106
meta: expect.anything(),
107107
});
108108
});
109+
110+
describe('null body status responses', () => {
111+
test('204 No Content response returns null', async () => {
112+
const callJsonApiFx = createJsonApiRequest({ request });
113+
114+
const fetchMock = vi
115+
.fn()
116+
.mockResolvedValue(new Response(null, { status: 204 }));
117+
118+
const scope = fork({ handlers: [[fetchFx, fetchMock]] });
119+
120+
const watcher = watchEffect(callJsonApiFx, scope);
121+
122+
await allSettled(callJsonApiFx, {
123+
scope,
124+
params: {},
125+
});
126+
127+
expect(watcher.listeners.onFailData).not.toBeCalled();
128+
expect(watcher.listeners.onDoneData).toBeCalledWith({
129+
result: null,
130+
meta: expect.anything(),
131+
});
132+
});
133+
134+
test('205 Reset Content response returns null', async () => {
135+
const callJsonApiFx = createJsonApiRequest({ request });
136+
137+
const fetchMock = vi
138+
.fn()
139+
.mockResolvedValue(new Response(null, { status: 205 }));
140+
141+
const scope = fork({ handlers: [[fetchFx, fetchMock]] });
142+
143+
const watcher = watchEffect(callJsonApiFx, scope);
144+
145+
await allSettled(callJsonApiFx, {
146+
scope,
147+
params: {},
148+
});
149+
150+
expect(watcher.listeners.onFailData).not.toBeCalled();
151+
expect(watcher.listeners.onDoneData).toBeCalledWith({
152+
result: null,
153+
meta: expect.anything(),
154+
});
155+
});
156+
});
109157
});

packages/core/src/fetch/api.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
formatUrl,
1616
mergeRecords,
1717
formatHeaders,
18+
isNullBodyStatus,
1819
type FetchApiRecord,
1920
mergeQueryStrings,
2021
} from './lib';
@@ -160,9 +161,15 @@ export function createApiRequest<
160161
throw { error: transformedError, responseMeta: cause.responseMeta };
161162
});
162163

163-
const [forPrepare, forError] = response.body?.tee() ?? [null, null];
164164
const responseHeaders = response.headers;
165165

166+
// For null body statuses (101, 103, 204, 205, 304), the Response constructor
167+
// throws if a body is provided, so we must use null body for these statuses.
168+
const hasNullBodyStatus = isNullBodyStatus(response.status);
169+
const [forPrepare, forError] = hasNullBodyStatus
170+
? [null, null]
171+
: (response.body?.tee() ?? [null, null]);
172+
166173
const prepared = await prepareFx(new Response(forPrepare, response)).then(
167174
async (result) => {
168175
await drain(forError);
@@ -172,7 +179,7 @@ export function createApiRequest<
172179
async (cause) => {
173180
throw {
174181
error: preparationError({
175-
response: await new Response(forError).text(),
182+
response: forError ? await new Response(forError).text() : '',
176183
reason: cause?.message ?? null,
177184
}),
178185
responseMeta: { headers: responseHeaders },

packages/core/src/fetch/json.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
ExclusiveRequestConfigShared,
1212
StaticOnlyRequestConfig,
1313
} from './api';
14-
import { mergeRecords } from './lib';
14+
import { isNullBodyStatus, mergeRecords } from './lib';
1515
import { drain } from 'libs/lohyphen';
1616

1717
export type JsonObject = Record<string, Json>;
@@ -109,6 +109,12 @@ export function createJsonApiRequest<R extends CreationRequestConfig>(
109109
async function checkEmptyResponse(
110110
response: Response
111111
): Promise<[true, null] | [false, Response]> {
112+
// Null body statuses (101, 103, 204, 205, 304) cannot have a body per the Fetch spec.
113+
// We must check this early to avoid "Response with null body status cannot have body" error.
114+
if (isNullBodyStatus(response.status)) {
115+
return [true, null];
116+
}
117+
112118
if (!response.body) {
113119
return [true, null];
114120
}

packages/core/src/fetch/lib.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
import { configurationError } from '../errors/create_error';
22

3+
/**
4+
* HTTP status codes that indicate a null body response.
5+
* Per the Fetch specification, responses with these status codes cannot have a body.
6+
* Attempting to construct a Response with a body for these statuses throws:
7+
* "TypeError: Response with null body status cannot have body"
8+
*
9+
* @see https://fetch.spec.whatwg.org/#null-body-status
10+
*/
11+
const NULL_BODY_STATUSES = new Set([101, 103, 204, 205, 304]);
12+
13+
export function isNullBodyStatus(status: number): boolean {
14+
return NULL_BODY_STATUSES.has(status);
15+
}
16+
317
export type FetchApiRecord = Record<
418
string,
519
string | string[] | number | boolean | null | undefined

packages/core/src/mutation/__tests__/create_json_mutation.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,61 @@ describe('createJsonMutation', () => {
156156
);
157157
});
158158

159+
test('handle 204 No Content response with DELETE method', async () => {
160+
const mutation = createJsonMutation({
161+
request: {
162+
url: 'https://api.salo.com/resource/123',
163+
method: 'DELETE' as const,
164+
},
165+
response: { contract: unknownContract },
166+
});
167+
168+
const scope = fork({
169+
handlers: [[fetchFx, () => new Response(null, { status: 204 })]],
170+
});
171+
172+
const { listeners } = watchRemoteOperation(mutation, scope);
173+
174+
await allSettled(mutation.start, { scope });
175+
176+
expect(listeners.onFailure).not.toHaveBeenCalled();
177+
expect(listeners.onSuccess).toHaveBeenCalled();
178+
expect(listeners.onSuccess).toHaveBeenCalledWith(
179+
expect.objectContaining({
180+
result: null,
181+
params: undefined,
182+
})
183+
);
184+
});
185+
186+
test('handle 204 No Content response with POST method', async () => {
187+
const mutation = createJsonMutation({
188+
request: {
189+
url: 'https://api.salo.com/resource',
190+
method: 'POST' as const,
191+
body: { data: 'test' },
192+
},
193+
response: { contract: unknownContract },
194+
});
195+
196+
const scope = fork({
197+
handlers: [[fetchFx, () => new Response(null, { status: 204 })]],
198+
});
199+
200+
const { listeners } = watchRemoteOperation(mutation, scope);
201+
202+
await allSettled(mutation.start, { scope });
203+
204+
expect(listeners.onFailure).not.toHaveBeenCalled();
205+
expect(listeners.onSuccess).toHaveBeenCalled();
206+
expect(listeners.onSuccess).toHaveBeenCalledWith(
207+
expect.objectContaining({
208+
result: null,
209+
params: undefined,
210+
})
211+
);
212+
});
213+
159214
test('cancel json mutation by external clock', async () => {
160215
const abort = createEvent();
161216

0 commit comments

Comments
 (0)