Skip to content

Commit 57ebe1d

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 57ebe1d

15 files changed

Lines changed: 338 additions & 58 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { ApiException, ApiExceptionCode } 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+
describe('Client', () => {
24+
let client: Client
25+
let fetchMock: ReturnType<typeof vi.fn>
26+
27+
beforeEach(() => {
28+
client = new Client()
29+
;(client as any).baseUrl = 'https://api.example.com'
30+
fetchMock = vi.fn()
31+
vi.stubGlobal('fetch', fetchMock)
32+
})
33+
34+
afterEach(() => {
35+
vi.unstubAllGlobals()
36+
vi.restoreAllMocks()
37+
})
38+
39+
describe('path-based moved conflicts', () => {
40+
it('should surface the moved conflict without retrying', async () => {
41+
fetchMock.mockResolvedValueOnce(makeMovedResponse('/project/john/demo/view'))
42+
43+
try {
44+
await client.post('/project/John/demo/view')
45+
throw new Error('expected moved conflict')
46+
} catch (e) {
47+
expect(e).toBeInstanceOf(ApiException)
48+
expect(e).toMatchObject({
49+
code: ApiExceptionCode.errorResourceMoved,
50+
meta: {
51+
path: '/project/john/demo/view'
52+
}
53+
})
54+
}
55+
56+
expect(fetchMock).toHaveBeenCalledTimes(1)
57+
expect(new URL((fetchMock.mock.calls[0]![0] as Request).url).pathname).toBe('/project/John/demo/view')
58+
})
59+
})
60+
})

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as Sentry from '@sentry/vue'
66
import { apiBaseUrl } from '@/utils/env'
77
import { TimeoutException } from '@/utils/exception/base'
88
import { mergeSignals } from '@/utils/disposable'
9-
import { ApiException } from './exception'
9+
import { ApiException, isMovedResourceCanonical } from './exception'
1010
import { parseSSE, type SSEEvent } from './sse'
1111

1212
/** Response body when exception encountered for API calling */
@@ -15,12 +15,18 @@ export type ApiExceptionPayload = {
1515
code: number
1616
/** Message for developer reading */
1717
msg: string
18+
canonical?: unknown
1819
}
1920

2021
function isApiExceptionPayload(body: any): body is ApiExceptionPayload {
2122
return body && typeof body.code === 'number' && typeof body.msg === 'string'
2223
}
2324

25+
function getMovedResourceCanonical(body: ApiExceptionPayload | undefined) {
26+
const canonical = body?.canonical
27+
return isMovedResourceCanonical(canonical) ? canonical : null
28+
}
29+
2430
/** TokenProvider provides access token used for the Authorization header */
2531
export type TokenProvider = () => Promise<string | null>
2632

@@ -83,8 +89,13 @@ export class Client {
8389
} catch {
8490
// ignore
8591
}
92+
const movedCanonical = getMovedResourceCanonical(payload)
8693
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 })
94+
throw new ApiException(payload.code, payload.msg, {
95+
req,
96+
resp,
97+
meta: movedCanonical ?? undefined
98+
})
8899
}
89100
return resp
90101
}

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

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ export class ApiException extends Exception {
1414
constructor(
1515
public code: number,
1616
message: string,
17-
{ req, resp }: { req: Request; resp: Response }
17+
{ req, resp, meta }: { req: Request; resp: Response; meta?: unknown }
1818
) {
1919
super(`[${code}] ${message} (${req.method} ${req.url.slice(0, 200)})`)
2020
this.userMessage = codeMessages[this.code as ApiExceptionCode] ?? null
21-
this.meta = codeMetas[this.code as ApiExceptionCode]?.(resp.headers) ?? null
21+
this.meta = meta ?? codeMetas[this.code as ApiExceptionCode]?.(resp.headers) ?? null
2222
}
2323
}
2424

@@ -28,12 +28,32 @@ export enum ApiExceptionCode {
2828
errorForbidden = 40300,
2929
errorQuotaExceeded = 40301,
3030
errorNotFound = 40400,
31+
errorResourceMoved = 40901,
3132
errorTooManyRequests = 42900,
3233
errorRateLimitExceeded = 42901,
3334
errorScratchFeatureNotSupported = 50101,
3435
errorUnknown = 50000
3536
}
3637

38+
export type MovedResourceCanonical = {
39+
path: string
40+
username?: string
41+
owner?: string
42+
name?: string
43+
release?: string
44+
}
45+
46+
export function isMovedResourceCanonical(meta: unknown): meta is MovedResourceCanonical {
47+
if (meta == null || typeof meta !== 'object') return false
48+
const canonical = meta as Record<string, unknown>
49+
if (typeof canonical.path !== 'string') return false
50+
if (canonical.username != null && typeof canonical.username !== 'string') return false
51+
if (canonical.owner != null && typeof canonical.owner !== 'string') return false
52+
if (canonical.name != null && typeof canonical.name !== 'string') return false
53+
if (canonical.release != null && typeof canonical.release !== 'string') return false
54+
return true
55+
}
56+
3757
export type QuotaExceededMeta = {
3858
// milliseconds or null
3959
retryAfter: number | null
@@ -72,6 +92,14 @@ const codeMessages: Record<ApiExceptionCode, LocaleMessage> = {
7292
en: 'quota exceeded',
7393
zh: '超出配额'
7494
},
95+
[ApiExceptionCode.errorNotFound]: {
96+
en: 'resource not exist',
97+
zh: '资源不存在'
98+
},
99+
[ApiExceptionCode.errorResourceMoved]: {
100+
en: 'resource moved',
101+
zh: '资源已迁移'
102+
},
75103
[ApiExceptionCode.errorTooManyRequests]: {
76104
en: 'too many requests',
77105
zh: '请求太频繁了'
@@ -80,10 +108,6 @@ const codeMessages: Record<ApiExceptionCode, LocaleMessage> = {
80108
en: 'rate limit exceeded, please retry later',
81109
zh: '触发频率限制,请稍后重试'
82110
},
83-
[ApiExceptionCode.errorNotFound]: {
84-
en: 'resource not exist',
85-
zh: '资源不存在'
86-
},
87111
[ApiExceptionCode.errorScratchFeatureNotSupported]: {
88112
en: 'Some Scratch features are not supported yet',
89113
zh: '部分 Scratch 特性暂不支持'

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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 { 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+
resp: new Response(null, { status: 409 }),
10+
meta: canonical
11+
})
12+
}
13+
14+
describe('updateProject', () => {
15+
let mockedPatch: ReturnType<typeof vi.spyOn>
16+
17+
beforeEach(() => {
18+
mockedPatch = vi.spyOn(client, 'patch')
19+
})
20+
21+
afterEach(() => {
22+
vi.restoreAllMocks()
23+
})
24+
25+
it('should retry once with the canonical project route', async () => {
26+
mockedPatch
27+
.mockRejectedValueOnce(makeMovedException({ path: '/project/john/demo', owner: 'john', name: 'demo' }))
28+
.mockResolvedValueOnce({
29+
owner: 'john',
30+
name: 'demo'
31+
})
32+
33+
const saved = await updateProject('John', 'Demo', { displayName: 'Demo' })
34+
35+
expect(saved).toMatchObject({
36+
owner: 'john',
37+
name: 'demo'
38+
})
39+
expect(mockedPatch).toHaveBeenCalledTimes(2)
40+
expect(mockedPatch.mock.calls[0]![0]).toBe('/project/John/Demo')
41+
expect(mockedPatch.mock.calls[1]![0]).toBe('/project/john/demo')
42+
})
43+
44+
it('should surface moved errors without canonical project route fields', async () => {
45+
const movedError = makeMovedException({ path: '/project/john/demo' })
46+
mockedPatch.mockRejectedValueOnce(movedError)
47+
48+
await expect(updateProject('John', 'Demo', { displayName: 'Demo' })).rejects.toBe(movedError)
49+
50+
expect(mockedPatch).toHaveBeenCalledTimes(1)
51+
})
52+
53+
it('should surface moved errors with empty canonical project route fields', async () => {
54+
const movedError = makeMovedException({ path: '/project/john/demo', owner: '', name: 'demo' })
55+
mockedPatch.mockRejectedValueOnce(movedError)
56+
57+
await expect(updateProject('John', 'Demo', { displayName: 'Demo' })).rejects.toBe(movedError)
58+
59+
expect(mockedPatch).toHaveBeenCalledTimes(1)
60+
})
61+
})

spx-gui/src/apis/project.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import dayjs from 'dayjs'
22
import type { FileCollection, ByPage, PaginationParams, Perspective, ArtStyle } from './common'
33
import { client, Visibility, ownerAll, timeStringify } from './common'
4-
import { ApiException, ApiExceptionCode } from './common/exception'
4+
import { ApiException, ApiExceptionCode, isMovedResourceCanonical } from './common/exception'
55
import { parseProjectReleaseFullName, stringifyProjectReleaseFullName, type ProjectRelease } from './project-release'
66
import type { Prettify } from '@/utils/types'
77

@@ -101,11 +101,34 @@ export type UpdateProjectParams = Prettify<
101101
>
102102

103103
export function updateProject(owner: string, name: string, params: UpdateProjectParams, signal?: AbortSignal) {
104+
return updateProjectOnce(owner, name, params, signal).catch((err) => {
105+
const movedProjectRoute = getMovedProjectRoute(err)
106+
if (movedProjectRoute == null) throw err
107+
108+
const { owner: canonicalOwner, name: canonicalName } = movedProjectRoute
109+
if (canonicalOwner === owner && canonicalName === name) throw err
110+
111+
return updateProjectOnce(canonicalOwner, canonicalName, params, signal)
112+
})
113+
}
114+
115+
function updateProjectOnce(owner: string, name: string, params: UpdateProjectParams, signal?: AbortSignal) {
104116
return client.patch(`/project/${encodeURIComponent(owner)}/${encodeURIComponent(name)}`, params, {
105117
signal
106118
}) as Promise<ProjectData>
107119
}
108120

121+
function getMovedProjectRoute(err: unknown): { owner: string; name: string } | null {
122+
if (!(err instanceof ApiException) || err.code !== ApiExceptionCode.errorResourceMoved) return null
123+
if (!isMovedResourceCanonical(err.meta)) return null
124+
if (err.meta.owner == null || err.meta.name == null) return null
125+
if (err.meta.owner === '' || err.meta.name === '') return null
126+
return {
127+
owner: err.meta.owner,
128+
name: err.meta.name
129+
}
130+
}
131+
109132
export function deleteProject(owner: string, name: string) {
110133
return client.delete(`/project/${encodeURIComponent(owner)}/${encodeURIComponent(name)}`) as Promise<void>
111134
}

0 commit comments

Comments
 (0)