Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -510,5 +510,62 @@ describe('FormData boundary handling', () => {
});
});

describe('non-2xx response handling', () => {
const client = createClient({ baseUrl: 'https://example.com' });

const makeMockOfetchWithError = (response: Response): MockOfetch => {
const fn: any = vi.fn();
const error: any = new Error('FetchError');
error.response = response;
fn.raw = vi.fn().mockRejectedValue(error);
return fn as MockOfetch;
};

it('response interceptors still run for 4xx responses', async () => {
const mockResponse = new Response(JSON.stringify({ message: 'Not Found' }), {
headers: { 'Content-Type': 'application/json' },
status: 404,
});

const mockOfetch = makeMockOfetchWithError(mockResponse);
const responseInterceptor = vi.fn().mockImplementation((r: Response) => r);
const interceptorId = client.interceptors.response.use(responseInterceptor);

await client.request({ method: 'GET', ofetch: mockOfetch as any, url: '/test' });

expect(responseInterceptor).toHaveBeenCalledOnce();
expect(responseInterceptor.mock.calls[0]![0]).toHaveProperty('status', 404);

client.interceptors.response.eject(interceptorId);
});

it('returns error payload when ofetch throws for 4xx', async () => {
const mockResponse = new Response(JSON.stringify({ message: 'Not Found' }), {
headers: { 'Content-Type': 'application/json' },
status: 404,
});
(mockResponse as any)._data = { message: 'Not Found' };

const mockOfetch = makeMockOfetchWithError(mockResponse);

const result = await client.request({ method: 'GET', ofetch: mockOfetch as any, url: '/test' });

expect(result.error).toEqual({ message: 'Not Found' });
expect(result.response?.status).toBe(404);
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no test asserting the opt-out path (ignoreResponseError: true) still works as a backward-compat escape hatch. A future change forcing the option to false regardless of user input would go unnoticed. Consider a fourth test that passes ignoreResponseError: true, uses mockResolvedValue(response404) (not rejected), and asserts the client returns { error, response } without entering the new catch branch.


it('handles network errors (no response) via error interceptors', async () => {
const fn: any = vi.fn();
fn.raw = vi.fn().mockRejectedValue(new Error('Network error'));
const mockOfetch = fn as MockOfetch;

const result = await client.request({ method: 'GET', ofetch: mockOfetch as any, url: '/test' });

expect(result.error).toBeInstanceOf(Error);
expect((result.error as Error).message).toBe('Network error');
expect(result.response).toBeUndefined();
});
});

// Note: дополнительные проверки поведения ofetch (responseType/responseStyle/retry)
// не дублируем, чтобы набор тестов оставался сопоставим с другими клиентами.
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,19 @@ export const createClient = (config: Config = {}): Client => {
// build ofetch options and perform the request (.raw keeps the Response)
const responseOptions = buildNetworkOptions(opts, networkBody, ofetchResponseType);

response = await $ofetch.raw(finalUrl, responseOptions);
try {
response = await $ofetch.raw(finalUrl, responseOptions);
} catch (fetchError: unknown) {
// When ignoreResponseError is false (default), ofetch throws FetchError for non-2xx.
// Extract the response so response interceptors can still process it before we
// decide whether to throw or return the error.
const maybeFetchError = fetchError as { response?: typeof response };
if (maybeFetchError?.response) {
response = maybeFetchError.response;
} else {
throw fetchError;
}
}
Comment on lines +183 to +195
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, but worth noting: real ofetch FetchError also exposes .status, .statusText, and .data as getters on the error itself (see unjs/ofetch src/error.ts). The mock in the new tests only mirrors .response, which is fine for what this block reads today but would invisibly drift from reality if fetchError.status-based branching (e.g. retry on specific codes) is added later. Not a change request — a note for whoever extends this catch.


for (const fn of interceptors.response.fns) {
if (fn) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ export const buildOfetchOptions = (
credentials: opts.credentials as OfetchOptions['credentials'],
dispatcher: opts.dispatcher as OfetchOptions['dispatcher'],
headers: opts.headers as Headers,
ignoreResponseError: (opts.ignoreResponseError as OfetchOptions['ignoreResponseError']) ?? true,
ignoreResponseError:
(opts.ignoreResponseError as OfetchOptions['ignoreResponseError']) ?? false,
method: opts.method,
onRequest: opts.onRequest as OfetchOptions['onRequest'],
onRequestError: opts.onRequestError as OfetchOptions['onRequestError'],
Expand Down Expand Up @@ -499,7 +500,7 @@ export const createConfig = <T extends ClientOptions = ClientOptions>(
): Config<Omit<ClientOptions, keyof T> & T> => ({
...jsonBodySerializer,
headers: defaultHeaders,
ignoreResponseError: true,
ignoreResponseError: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The central behavioral change of this PR is the flip from true to false here and at line 302, but no added test observes either value. A regression reverting one (or a typo letting the two defaults diverge) would pass the new suite because every new test uses mockRejectedValue(...) and never inspects the ofetch options. Consider adding expect(createClient({}).getConfig().ignoreResponseError).toBe(false) and/or a test that captures mockOfetch.raw.mock.calls[0][1].ignoreResponseError — the existing serialized request body handling suite already uses that pattern.

parseAs: 'auto',
querySerializer: defaultQuerySerializer,
...override,
Expand Down
Loading