Skip to content

Commit 083ea6e

Browse files
committed
fix: handle canonicalized route responses
Handle moved-resource responses from the backend without generic write retries, and retry updateProject once with the canonical route so saving existing projects still works after a project rename. Replace unresolved user, project, and editor routes with canonical backend values after loading so subsequent UI behavior uses canonical identifiers consistently. Updates #2993 Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
1 parent 882f405 commit 083ea6e

18 files changed

Lines changed: 536 additions & 90 deletions

File tree

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { ApiException, ApiExceptionCode, isQuotaExceededMeta } from './exception'
3+
import { Client } from './client'
4+
5+
function makeMovedResponse(canonicalPath: string) {
6+
return new Response(
7+
JSON.stringify({
8+
code: ApiExceptionCode.errorResourceMoved,
9+
msg: 'Resource moved',
10+
canonical: {
11+
path: canonicalPath
12+
}
13+
}),
14+
{
15+
status: 409,
16+
headers: {
17+
'Content-Type': 'application/json'
18+
}
19+
}
20+
)
21+
}
22+
23+
function makeQuotaExceededResponse(retryAfter: string) {
24+
return new Response(
25+
JSON.stringify({
26+
code: ApiExceptionCode.errorQuotaExceeded,
27+
msg: 'Quota exceeded'
28+
}),
29+
{
30+
status: 403,
31+
headers: {
32+
'Content-Type': 'application/json',
33+
'Retry-After': retryAfter
34+
}
35+
}
36+
)
37+
}
38+
39+
describe('Client', () => {
40+
let client: Client
41+
let fetchMock: ReturnType<typeof vi.fn>
42+
43+
beforeEach(() => {
44+
client = new Client()
45+
;(client as any).baseUrl = 'https://api.example.com'
46+
fetchMock = vi.fn()
47+
vi.stubGlobal('fetch', fetchMock)
48+
})
49+
50+
afterEach(() => {
51+
vi.unstubAllGlobals()
52+
vi.restoreAllMocks()
53+
})
54+
55+
describe('path-based moved conflicts', () => {
56+
it('should surface the moved conflict without retrying', async () => {
57+
fetchMock.mockResolvedValueOnce(makeMovedResponse('/project/john/demo/view'))
58+
59+
try {
60+
await client.post('/project/John/demo/view')
61+
throw new Error('expected moved conflict')
62+
} catch (e) {
63+
expect(e).toBeInstanceOf(ApiException)
64+
expect(e).toMatchObject({
65+
code: ApiExceptionCode.errorResourceMoved,
66+
meta: {
67+
path: '/project/john/demo/view'
68+
}
69+
})
70+
}
71+
72+
expect(fetchMock).toHaveBeenCalledTimes(1)
73+
expect(new URL((fetchMock.mock.calls[0]![0] as Request).url).pathname).toBe('/project/John/demo/view')
74+
})
75+
})
76+
77+
describe('quota exceeded metadata', () => {
78+
it('should parse retry-after metadata from headers', async () => {
79+
const retryAfter = 'Wed, 09 Apr 2026 08:00:00 GMT'
80+
fetchMock.mockResolvedValueOnce(makeQuotaExceededResponse(retryAfter))
81+
82+
try {
83+
await client.get('/quota')
84+
throw new Error('expected quota exceeded error')
85+
} catch (e) {
86+
expect(e).toBeInstanceOf(ApiException)
87+
expect(e).toMatchObject({
88+
code: ApiExceptionCode.errorQuotaExceeded
89+
})
90+
expect(isQuotaExceededMeta((e as ApiException).code, (e as ApiException).meta)).toBe(true)
91+
expect((e as ApiException).meta).toMatchObject({
92+
retryAfter: new Date(retryAfter).valueOf()
93+
})
94+
}
95+
})
96+
})
97+
})

spx-gui/src/apis/common/client.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
*/
44

55
import * as Sentry from '@sentry/vue'
6+
import dayjs from 'dayjs'
67
import { apiBaseUrl } from '@/utils/env'
78
import { TimeoutException } from '@/utils/exception/base'
89
import { mergeSignals } from '@/utils/disposable'
9-
import { ApiException } from './exception'
10+
import { ApiException, ApiExceptionCode, type MovedResourceCanonical, type QuotaExceededMeta } from './exception'
1011
import { parseSSE, type SSEEvent } from './sse'
1112

1213
/** Response body when exception encountered for API calling */
@@ -15,12 +16,36 @@ export type ApiExceptionPayload = {
1516
code: number
1617
/** Message for developer reading */
1718
msg: string
19+
canonical?: MovedResourceCanonical
1820
}
1921

2022
function isApiExceptionPayload(body: any): body is ApiExceptionPayload {
2123
return body && typeof body.code === 'number' && typeof body.msg === 'string'
2224
}
2325

26+
function getQuotaExceededMeta(headers: Headers): QuotaExceededMeta {
27+
const retryAfter = headers.get('Retry-After')
28+
let date
29+
if (retryAfter != null) {
30+
const seconds = Number(retryAfter)
31+
date = Number.isFinite(seconds) ? dayjs().add(seconds, 's') : dayjs(retryAfter)
32+
}
33+
return {
34+
retryAfter: date?.isValid() ? date.valueOf() : null
35+
}
36+
}
37+
38+
function getApiExceptionMeta(code: number, resp: Response, payload: ApiExceptionPayload): unknown {
39+
switch (code) {
40+
case ApiExceptionCode.errorQuotaExceeded:
41+
return getQuotaExceededMeta(resp.headers)
42+
case ApiExceptionCode.errorResourceMoved:
43+
return payload.canonical ?? null
44+
default:
45+
return null
46+
}
47+
}
48+
2449
/** TokenProvider provides access token used for the Authorization header */
2550
export type TokenProvider = () => Promise<string | null>
2651

@@ -84,7 +109,10 @@ export class Client {
84109
// ignore
85110
}
86111
if (payload == null) throw new Error(`status ${resp.status} for api call: ${req.url.slice(0, 200)}`)
87-
throw new ApiException(payload.code, payload.msg, { req, resp })
112+
throw new ApiException(payload.code, payload.msg, {
113+
req,
114+
meta: getApiExceptionMeta(payload.code, resp, payload)
115+
})
88116
}
89117
return resp
90118
}

spx-gui/src/apis/common/exception.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import type { LocaleMessage } from '@/utils/i18n'
66
import { Exception } from '@/utils/exception'
7-
import dayjs from 'dayjs'
87

98
export class ApiException extends Exception {
109
name = 'ApiException'
@@ -14,11 +13,11 @@ export class ApiException extends Exception {
1413
constructor(
1514
public code: number,
1615
message: string,
17-
{ req, resp }: { req: Request; resp: Response }
16+
{ req, meta }: { req: Request; meta?: unknown }
1817
) {
1918
super(`[${code}] ${message} (${req.method} ${req.url.slice(0, 200)})`)
2019
this.userMessage = codeMessages[this.code as ApiExceptionCode] ?? null
21-
this.meta = codeMetas[this.code as ApiExceptionCode]?.(resp.headers) ?? null
20+
this.meta = meta ?? null
2221
}
2322
}
2423

@@ -28,29 +27,26 @@ export enum ApiExceptionCode {
2827
errorForbidden = 40300,
2928
errorQuotaExceeded = 40301,
3029
errorNotFound = 40400,
30+
errorResourceMoved = 40901,
3131
errorTooManyRequests = 42900,
3232
errorRateLimitExceeded = 42901,
3333
errorScratchFeatureNotSupported = 50101,
3434
errorUnknown = 50000
3535
}
3636

37+
export type MovedResourceCanonical = {
38+
path: string
39+
username?: string
40+
owner?: string
41+
name?: string
42+
release?: string
43+
}
44+
3745
export type QuotaExceededMeta = {
3846
// milliseconds or null
3947
retryAfter: number | null
4048
}
41-
const codeMetas: { [key in ApiExceptionCode]?: (headers: Headers) => unknown } = {
42-
[ApiExceptionCode.errorQuotaExceeded]: (headers): QuotaExceededMeta => {
43-
const retryAfter = headers.get('Retry-After')
44-
let date
45-
if (retryAfter != null) {
46-
const seconds = Number(retryAfter)
47-
date = Number.isFinite(seconds) ? dayjs().add(seconds, 's') : dayjs(retryAfter)
48-
}
49-
return {
50-
retryAfter: date?.isValid() ? date.valueOf() : null
51-
}
52-
}
53-
}
49+
5450
export function isQuotaExceededMeta(code: number, meta: unknown): meta is QuotaExceededMeta {
5551
return code === ApiExceptionCode.errorQuotaExceeded && meta != null
5652
}
@@ -72,6 +68,14 @@ const codeMessages: Record<ApiExceptionCode, LocaleMessage> = {
7268
en: 'quota exceeded',
7369
zh: '超出配额'
7470
},
71+
[ApiExceptionCode.errorNotFound]: {
72+
en: 'resource not exist',
73+
zh: '资源不存在'
74+
},
75+
[ApiExceptionCode.errorResourceMoved]: {
76+
en: 'resource moved',
77+
zh: '资源已迁移'
78+
},
7579
[ApiExceptionCode.errorTooManyRequests]: {
7680
en: 'too many requests',
7781
zh: '请求太频繁了'
@@ -80,10 +84,6 @@ const codeMessages: Record<ApiExceptionCode, LocaleMessage> = {
8084
en: 'rate limit exceeded, please retry later',
8185
zh: '触发频率限制,请稍后重试'
8286
},
83-
[ApiExceptionCode.errorNotFound]: {
84-
en: 'resource not exist',
85-
zh: '资源不存在'
86-
},
8787
[ApiExceptionCode.errorScratchFeatureNotSupported]: {
8888
en: 'Some Scratch features are not supported yet',
8989
zh: '部分 Scratch 特性暂不支持'

spx-gui/src/apis/project.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { client } from './common'
3+
import { ApiException, ApiExceptionCode, type MovedResourceCanonical } from './common/exception'
4+
import { isProjectNameTaken, updateProject } from './project'
5+
6+
function makeMovedException(canonical: MovedResourceCanonical) {
7+
return new ApiException(ApiExceptionCode.errorResourceMoved, 'Resource moved', {
8+
req: new Request('https://api.example.com/project/John/Demo', { method: 'PATCH' }),
9+
meta: canonical
10+
})
11+
}
12+
13+
describe('updateProject', () => {
14+
let mockedPatch: ReturnType<typeof vi.spyOn>
15+
16+
beforeEach(() => {
17+
mockedPatch = vi.spyOn(client, 'patch')
18+
})
19+
20+
afterEach(() => {
21+
vi.restoreAllMocks()
22+
})
23+
24+
it('should retry once with the canonical project route', async () => {
25+
mockedPatch
26+
.mockRejectedValueOnce(makeMovedException({ path: '/project/john/demo', owner: 'john', name: 'demo' }))
27+
.mockResolvedValueOnce({
28+
owner: 'john',
29+
name: 'demo'
30+
})
31+
32+
const saved = await updateProject('John', 'Demo', { displayName: 'Demo' })
33+
34+
expect(saved).toMatchObject({
35+
owner: 'john',
36+
name: 'demo'
37+
})
38+
expect(mockedPatch).toHaveBeenCalledTimes(2)
39+
expect(mockedPatch.mock.calls[0]![0]).toBe('/project/John/Demo')
40+
expect(mockedPatch.mock.calls[1]![0]).toBe('/project/john/demo')
41+
})
42+
43+
it('should surface moved errors without canonical project route fields', async () => {
44+
const movedError = makeMovedException({ path: '/project/john/demo' })
45+
mockedPatch.mockRejectedValueOnce(movedError)
46+
47+
await expect(updateProject('John', 'Demo', { displayName: 'Demo' })).rejects.toBe(movedError)
48+
49+
expect(mockedPatch).toHaveBeenCalledTimes(1)
50+
})
51+
52+
it('should surface moved errors with empty canonical project route fields', async () => {
53+
const movedError = makeMovedException({ path: '/project/john/demo', owner: '', name: 'demo' })
54+
mockedPatch.mockRejectedValueOnce(movedError)
55+
56+
await expect(updateProject('John', 'Demo', { displayName: 'Demo' })).rejects.toBe(movedError)
57+
58+
expect(mockedPatch).toHaveBeenCalledTimes(1)
59+
})
60+
})
61+
62+
describe('isProjectNameTaken', () => {
63+
let mockedGet: ReturnType<typeof vi.spyOn>
64+
65+
beforeEach(() => {
66+
mockedGet = vi.spyOn(client, 'get')
67+
})
68+
69+
afterEach(() => {
70+
vi.restoreAllMocks()
71+
})
72+
73+
it('should return true for a canonical project route hit', async () => {
74+
mockedGet.mockResolvedValueOnce({
75+
owner: 'john',
76+
name: 'demo'
77+
})
78+
79+
await expect(isProjectNameTaken('john', 'demo')).resolves.toBe(true)
80+
})
81+
82+
it('should return true when the canonical project route only differs by casing', async () => {
83+
mockedGet.mockResolvedValueOnce({
84+
owner: 'john',
85+
name: 'demo'
86+
})
87+
88+
await expect(isProjectNameTaken('John', 'Demo')).resolves.toBe(true)
89+
})
90+
91+
it('should return false for an alias hit that resolves to another canonical project name', async () => {
92+
mockedGet.mockResolvedValueOnce({
93+
owner: 'john',
94+
name: 'renamed-demo'
95+
})
96+
97+
await expect(isProjectNameTaken('john', 'demo')).resolves.toBe(false)
98+
})
99+
100+
it('should return false when the project route is not found', async () => {
101+
mockedGet.mockRejectedValueOnce(
102+
new ApiException(ApiExceptionCode.errorNotFound, 'Not found', {
103+
req: new Request('https://api.example.com/project/john/demo', { method: 'GET' })
104+
})
105+
)
106+
107+
await expect(isProjectNameTaken('john', 'demo')).resolves.toBe(false)
108+
})
109+
})

0 commit comments

Comments
 (0)