Skip to content

Commit 7aadf78

Browse files
committed
redirect: 'manual'
Signed-off-by: Carina Ursu <carina@union.ai>
1 parent e6c304d commit 7aadf78

2 files changed

Lines changed: 118 additions & 1 deletion

File tree

packages/oss-console/src/components/data/fetchClient.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ function queueRefresh(fn: () => Promise<void>): Promise<void> {
2727
return refreshInFlight;
2828
}
2929

30+
function isRedirectResponse(res: Response): boolean {
31+
if (res.type === 'opaqueredirect') {
32+
return true;
33+
}
34+
const { status } = res;
35+
// 304 Not Modified is 3xx but not a redirect; treat other 3xx like axios maxRedirects:0.
36+
return status >= 300 && status < 400 && status !== 304;
37+
}
38+
3039
function appendQuery(url: string, params?: Record<string, unknown>): string {
3140
if (!params) return url;
3241
const entries = Object.entries(params).filter(([, v]) => v !== undefined && v !== null);
@@ -78,7 +87,7 @@ export const fetchClient = {
7887
const init: FetchRequestInit = {
7988
method,
8089
credentials: 'include',
81-
redirect: 'error',
90+
redirect: 'manual',
8291
headers: config.headers,
8392
};
8493
if (method !== 'GET' && method !== 'HEAD' && config.data != null) {
@@ -89,6 +98,13 @@ export const fetchClient = {
8998

9099
let res = await doFetch();
91100

101+
if (isRedirectResponse(res)) {
102+
throw new HttpRequestError(res.statusText || 'Redirect', {
103+
status: res.status,
104+
statusText: res.statusText || 'Redirect',
105+
});
106+
}
107+
92108
if (res.status === 401 && !skipRefresh) {
93109
await queueRefresh(async () => {
94110
await refreshAuth(
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import { fetchClient } from '../fetchClient';
2+
3+
function buildResponse(overrides: Partial<Response> & Pick<Response, 'status'>): Response {
4+
const { status } = overrides;
5+
return {
6+
ok: status >= 200 && status < 300,
7+
statusText: overrides.statusText ?? '',
8+
type: 'basic',
9+
url: 'http://localhost/api',
10+
redirected: false,
11+
headers: new Headers(),
12+
arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)),
13+
...overrides,
14+
} as Response;
15+
}
16+
17+
describe('fetchClient.request', () => {
18+
const mockFetch = global.fetch as jest.MockedFunction<typeof fetch>;
19+
20+
beforeEach(() => {
21+
mockFetch.mockReset();
22+
mockFetch.mockImplementation(() =>
23+
Promise.resolve(
24+
buildResponse({
25+
status: 200,
26+
statusText: 'OK',
27+
ok: true,
28+
}),
29+
),
30+
);
31+
});
32+
33+
it('calls fetch with redirect: manual', async () => {
34+
await fetchClient.request({ url: 'http://localhost/api' });
35+
expect(mockFetch).toHaveBeenCalledWith(
36+
'http://localhost/api',
37+
expect.objectContaining({ redirect: 'manual' }),
38+
);
39+
});
40+
41+
it('throws HttpRequestError with status for 3xx redirect responses', async () => {
42+
mockFetch.mockImplementationOnce(() =>
43+
Promise.resolve(
44+
buildResponse({
45+
status: 302,
46+
statusText: 'Found',
47+
ok: false,
48+
headers: new Headers({ Location: '/login' }),
49+
}),
50+
),
51+
);
52+
53+
await expect(
54+
fetchClient.request({ url: 'http://localhost/api', skipAuthRefresh: true }),
55+
).rejects.toMatchObject({
56+
name: 'HttpRequestError',
57+
response: { status: 302, statusText: 'Found' },
58+
});
59+
});
60+
61+
it('throws HttpRequestError for opaqueredirect (cross-origin redirect) instead of TypeError', async () => {
62+
mockFetch.mockImplementationOnce(() =>
63+
Promise.resolve(
64+
buildResponse({
65+
status: 0,
66+
statusText: '',
67+
ok: false,
68+
type: 'opaqueredirect',
69+
}),
70+
),
71+
);
72+
73+
await expect(
74+
fetchClient.request({ url: 'http://localhost/api', skipAuthRefresh: true }),
75+
).rejects.toMatchObject({
76+
name: 'HttpRequestError',
77+
message: 'Redirect',
78+
response: { status: 0, statusText: 'Redirect' },
79+
});
80+
});
81+
82+
it('does not treat 304 Not Modified as a redirect error', async () => {
83+
mockFetch.mockImplementationOnce(() =>
84+
Promise.resolve(
85+
buildResponse({
86+
status: 304,
87+
statusText: 'Not Modified',
88+
ok: false,
89+
}),
90+
),
91+
);
92+
93+
await expect(
94+
fetchClient.request({ url: 'http://localhost/api', skipAuthRefresh: true }),
95+
).rejects.toMatchObject({
96+
name: 'HttpRequestError',
97+
message: 'Not Modified',
98+
response: expect.objectContaining({ status: 304 }),
99+
});
100+
});
101+
});

0 commit comments

Comments
 (0)