Skip to content

Commit ec6d86d

Browse files
dinwwwhdinwwwh
authored
fix(rpc,openapi): prevent lazy router races under concurrent requests (#1734)
## Problem When multiple requests hit the server at the same time while a lazy router was still loading, the matchers (`StandardRPCMatcher` and `StandardOpenAPIMatcher`) could: - Invoke the same lazy loader multiple times, once per concurrent request. - Lose routes entirely when a lazy router contained another lazy router — the nested route could then 404 permanently, even for later requests. ## Fix Concurrent requests now share a single load per lazy router: each loader runs exactly once, requests arriving mid-load wait for it instead of reloading or missing the route, and nested lazy routers always resolve correctly. If a loader fails, the request rejects and the next request retries as before. ## Tests New tests in both matchers cover concurrent requests to lazy routers, lazy routers nested inside lazy routers (up to three levels), multiple lazy branches matched in parallel, and retry after loader failure. All of them fail on the previous implementation. Full `server` and `openapi` suites pass (808 tests).
1 parent a8dd6da commit ec6d86d

4 files changed

Lines changed: 352 additions & 29 deletions

File tree

‎packages/openapi/src/adapters/standard/openapi-matcher.test.ts‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,147 @@ describe('standardOpenAPIMatcher', () => {
185185
expect(pongLoader).toHaveBeenCalledTimes(4)
186186
})
187187

188+
it('lazy router inside lazy router with concurrent requests', async () => {
189+
const base = os.$context<any>()
190+
191+
let resolveNested!: (value: { default: any }) => void
192+
const nestedLoader = vi.fn(() => new Promise<{ default: any }>((resolve) => {
193+
resolveNested = resolve
194+
}))
195+
const pongLoader = vi.fn(() => Promise.resolve({ default: pong }))
196+
197+
const rpcMatcher = new StandardOpenAPIMatcher()
198+
rpcMatcher.init(base.router({
199+
nested: base.lazy(nestedLoader),
200+
}))
201+
202+
const match1 = rpcMatcher.match('POST', '/nested/pong')
203+
const match2 = rpcMatcher.match('POST', '/nested/pong')
204+
205+
expect(nestedLoader).toHaveBeenCalledTimes(1)
206+
207+
resolveNested({ default: { pong: base.lazy(pongLoader) } })
208+
209+
expect(await match1).toEqual({
210+
path: ['nested', 'pong'],
211+
procedure: pong,
212+
})
213+
214+
expect(await match2).toEqual({
215+
path: ['nested', 'pong'],
216+
procedure: pong,
217+
})
218+
219+
expect(nestedLoader).toHaveBeenCalledTimes(1)
220+
expect(pongLoader).toHaveBeenCalledTimes(1)
221+
222+
// subsequent requests still work without reloading
223+
expect(await rpcMatcher.match('POST', '/nested/pong')).toEqual({
224+
path: ['nested', 'pong'],
225+
procedure: pong,
226+
})
227+
228+
expect(nestedLoader).toHaveBeenCalledTimes(1)
229+
expect(pongLoader).toHaveBeenCalledTimes(1)
230+
})
231+
232+
it('multiple lazy routers with lazy router inside lazy router', async () => {
233+
const base = os.$context<any>()
234+
235+
const userFindLoader = vi.fn(() => Promise.resolve({ default: pong }))
236+
const usersLoader = vi.fn(() => Promise.resolve({
237+
default: {
238+
find: base.lazy(userFindLoader),
239+
},
240+
}))
241+
242+
const planetDeepLoader = vi.fn(() => Promise.resolve({ default: pong }))
243+
const planetNestedLoader = vi.fn(() => Promise.resolve({
244+
default: {
245+
deep: base.lazy(planetDeepLoader),
246+
},
247+
}))
248+
const planetsLoader = vi.fn(() => Promise.resolve({
249+
default: {
250+
nested: base.lazy(planetNestedLoader),
251+
pong,
252+
},
253+
}))
254+
255+
const unusedLoader = vi.fn(() => Promise.resolve({ default: pong }))
256+
257+
const rpcMatcher = new StandardOpenAPIMatcher()
258+
rpcMatcher.init(base.router({
259+
users: base.prefix('/users').lazy(usersLoader),
260+
planets: base.prefix('/planets').lazy(planetsLoader),
261+
unused: base.prefix('/unused').lazy(unusedLoader),
262+
}))
263+
264+
// concurrent requests across different lazy branches
265+
const [match1, match2, match3] = await Promise.all([
266+
rpcMatcher.match('POST', '/users/find'),
267+
rpcMatcher.match('POST', '/planets/nested/deep'),
268+
rpcMatcher.match('POST', '/planets/pong'),
269+
])
270+
271+
expect(match1).toEqual({
272+
path: ['users', 'find'],
273+
procedure: pong,
274+
})
275+
276+
expect(match2).toEqual({
277+
path: ['planets', 'nested', 'deep'],
278+
procedure: pong,
279+
})
280+
281+
expect(match3).toEqual({
282+
path: ['planets', 'pong'],
283+
procedure: pong,
284+
})
285+
286+
expect(usersLoader).toHaveBeenCalledTimes(1)
287+
expect(userFindLoader).toHaveBeenCalledTimes(1)
288+
expect(planetsLoader).toHaveBeenCalledTimes(1)
289+
expect(planetNestedLoader).toHaveBeenCalledTimes(1)
290+
expect(planetDeepLoader).toHaveBeenCalledTimes(1)
291+
292+
// prefixed router not matched by any request stays lazy
293+
expect(unusedLoader).toHaveBeenCalledTimes(0)
294+
295+
// subsequent requests still work without reloading
296+
expect(await rpcMatcher.match('POST', '/planets/nested/deep')).toEqual({
297+
path: ['planets', 'nested', 'deep'],
298+
procedure: pong,
299+
})
300+
301+
expect(planetsLoader).toHaveBeenCalledTimes(1)
302+
expect(planetNestedLoader).toHaveBeenCalledTimes(1)
303+
expect(planetDeepLoader).toHaveBeenCalledTimes(1)
304+
expect(unusedLoader).toHaveBeenCalledTimes(0)
305+
})
306+
307+
it('lazy router can retry after loader failure', async () => {
308+
const base = os.$context<any>()
309+
310+
const pongLoader = vi.fn()
311+
.mockRejectedValueOnce(new Error('loader failed'))
312+
.mockResolvedValueOnce({ default: pong })
313+
314+
const rpcMatcher = new StandardOpenAPIMatcher()
315+
rpcMatcher.init(base.router({
316+
pong: base.lazy(pongLoader),
317+
}))
318+
319+
await expect(rpcMatcher.match('POST', '/pong')).rejects.toThrow('loader failed')
320+
321+
expect(await rpcMatcher.match('POST', '/pong')).toEqual({
322+
path: ['pong'],
323+
procedure: pong,
324+
})
325+
326+
expect(pongLoader).toHaveBeenCalledTimes(2)
327+
})
328+
188329
it('/ in path', async () => {
189330
const ping1 = new Procedure({
190331
...ping['~orpc'],

‎packages/openapi/src/adapters/standard/openapi-matcher.ts‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ export class StandardOpenAPIMatcher implements StandardMatcher {
2929
router: AnyRouter
3030
}>()
3131

32-
private pendingRouters: (LazyTraverseContractProceduresOptions & { httpPathPrefix: HTTPPath, laziedPrefix: string | undefined }) [] = []
32+
private readonly pendingRouters: (LazyTraverseContractProceduresOptions & {
33+
httpPathPrefix: HTTPPath
34+
laziedPrefix: string | undefined
35+
initPromise?: Promise<void>
36+
}) [] = []
3337

3438
constructor(options: StandardOpenAPIMatcherOptions = {}) {
3539
this.filter = options.filter ?? true
@@ -72,24 +76,35 @@ export class StandardOpenAPIMatcher implements StandardMatcher {
7276
}
7377

7478
async match(method: string, pathname: HTTPPath): Promise<StandardMatchResult> {
75-
if (this.pendingRouters.length) {
76-
const newPendingRouters: typeof this.pendingRouters = []
77-
78-
for (const pendingRouter of this.pendingRouters) {
79-
if (
80-
!pendingRouter.laziedPrefix
79+
/**
80+
* Resolve pending routers one at a time and re-scan after each, so lazy routers
81+
* nested inside a just-initialized lazy router are also picked up.
82+
*/
83+
while (true) {
84+
const pendingRouter = this.pendingRouters.find(
85+
pendingRouter => !pendingRouter.laziedPrefix
8186
|| pathname.startsWith(pendingRouter.laziedPrefix)
82-
|| pathname.startsWith(pendingRouter.httpPathPrefix)
83-
) {
84-
const { default: router } = await unlazy(pendingRouter.router)
85-
this.init(router, pendingRouter.path)
86-
}
87-
else {
88-
newPendingRouters.push(pendingRouter)
89-
}
87+
|| pathname.startsWith(pendingRouter.httpPathPrefix),
88+
)
89+
90+
if (!pendingRouter) {
91+
break
9092
}
9193

92-
this.pendingRouters = newPendingRouters
94+
/**
95+
* Memoize the init work and only remove the pending router after it finishes,
96+
* so concurrent requests share a single init instead of re-initializing
97+
* the same router or dropping pending routers pushed by each other.
98+
*/
99+
pendingRouter.initPromise ??= unlazy(pendingRouter.router).then(({ default: router }) => {
100+
this.init(router, pendingRouter.path)
101+
this.pendingRouters.splice(this.pendingRouters.indexOf(pendingRouter), 1)
102+
}).catch((error) => {
103+
pendingRouter.initPromise = undefined // allow retrying on failure
104+
throw error
105+
})
106+
107+
await pendingRouter.initPromise
93108
}
94109

95110
const match = findRoute(this.tree, method, pathname)

‎packages/server/src/adapters/standard/rpc-matcher.test.ts‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,157 @@ describe('standardRPCMatcher', () => {
169169
expect(pongLoader).toHaveBeenCalledTimes(4)
170170
})
171171

172+
it('lazy router inside lazy router with concurrent requests', async () => {
173+
const base = os.$context<any>()
174+
175+
let resolveNested!: (value: { default: any }) => void
176+
const nestedLoader = vi.fn(() => new Promise<{ default: any }>((resolve) => {
177+
resolveNested = resolve
178+
}))
179+
const pingLoader = vi.fn(() => Promise.resolve({ default: ping }))
180+
181+
const rpcMatcher = new StandardRPCMatcher()
182+
rpcMatcher.init(base.router({
183+
pong,
184+
nested: base.lazy(nestedLoader),
185+
}))
186+
187+
const match1 = rpcMatcher.match('POST', '/nested/ping')
188+
const match2 = rpcMatcher.match('POST', '/nested/ping')
189+
// non-matching request should not wait for or drop pending lazy routers
190+
const match3 = rpcMatcher.match('POST', '/pong')
191+
192+
expect(nestedLoader).toHaveBeenCalledTimes(1)
193+
194+
expect(await match3).toEqual({
195+
path: ['pong'],
196+
procedure: pong,
197+
})
198+
199+
resolveNested({ default: { ping: base.lazy(pingLoader) } })
200+
201+
expect(await match1).toEqual({
202+
path: ['nested', 'ping'],
203+
procedure: ping,
204+
})
205+
206+
expect(await match2).toEqual({
207+
path: ['nested', 'ping'],
208+
procedure: ping,
209+
})
210+
211+
expect(nestedLoader).toHaveBeenCalledTimes(1)
212+
expect(pingLoader).toHaveBeenCalledTimes(1)
213+
214+
// subsequent requests still work without reloading
215+
expect(await rpcMatcher.match('POST', '/nested/ping')).toEqual({
216+
path: ['nested', 'ping'],
217+
procedure: ping,
218+
})
219+
220+
expect(nestedLoader).toHaveBeenCalledTimes(1)
221+
expect(pingLoader).toHaveBeenCalledTimes(1)
222+
})
223+
224+
it('multiple lazy routers with lazy router inside lazy router', async () => {
225+
const base = os.$context<any>()
226+
227+
const userFindLoader = vi.fn(() => Promise.resolve({ default: ping }))
228+
const userListLoader = vi.fn(() => Promise.resolve({ default: pong }))
229+
const usersLoader = vi.fn(() => Promise.resolve({
230+
default: {
231+
find: base.lazy(userFindLoader),
232+
list: base.lazy(userListLoader),
233+
},
234+
}))
235+
236+
const planetDeepLoader = vi.fn(() => Promise.resolve({ default: ping }))
237+
const planetNestedLoader = vi.fn(() => Promise.resolve({
238+
default: {
239+
deep: base.lazy(planetDeepLoader),
240+
},
241+
}))
242+
const planetsLoader = vi.fn(() => Promise.resolve({
243+
default: {
244+
nested: base.lazy(planetNestedLoader),
245+
pong,
246+
},
247+
}))
248+
249+
const unusedLoader = vi.fn(() => Promise.resolve({ default: ping }))
250+
251+
const rpcMatcher = new StandardRPCMatcher()
252+
rpcMatcher.init(base.router({
253+
users: base.lazy(usersLoader),
254+
planets: base.lazy(planetsLoader),
255+
unused: base.lazy(unusedLoader),
256+
}))
257+
258+
// concurrent requests across different lazy branches
259+
const [match1, match2, match3] = await Promise.all([
260+
rpcMatcher.match('POST', '/users/find'),
261+
rpcMatcher.match('POST', '/planets/nested/deep'),
262+
rpcMatcher.match('POST', '/planets/pong'),
263+
])
264+
265+
expect(match1).toEqual({
266+
path: ['users', 'find'],
267+
procedure: ping,
268+
})
269+
270+
expect(match2).toEqual({
271+
path: ['planets', 'nested', 'deep'],
272+
procedure: ping,
273+
})
274+
275+
expect(match3).toEqual({
276+
path: ['planets', 'pong'],
277+
procedure: pong,
278+
})
279+
280+
expect(usersLoader).toHaveBeenCalledTimes(1)
281+
expect(userFindLoader).toHaveBeenCalledTimes(1)
282+
expect(planetsLoader).toHaveBeenCalledTimes(1)
283+
expect(planetNestedLoader).toHaveBeenCalledTimes(1)
284+
expect(planetDeepLoader).toHaveBeenCalledTimes(1)
285+
286+
// routers not matched by any request stay lazy
287+
expect(userListLoader).toHaveBeenCalledTimes(0)
288+
expect(unusedLoader).toHaveBeenCalledTimes(0)
289+
290+
// remaining pending routers still resolvable afterwards
291+
expect(await rpcMatcher.match('POST', '/users/list')).toEqual({
292+
path: ['users', 'list'],
293+
procedure: pong,
294+
})
295+
296+
expect(usersLoader).toHaveBeenCalledTimes(1)
297+
expect(userListLoader).toHaveBeenCalledTimes(1)
298+
expect(unusedLoader).toHaveBeenCalledTimes(0)
299+
})
300+
301+
it('lazy router can retry after loader failure', async () => {
302+
const base = os.$context<any>()
303+
304+
const pingLoader = vi.fn()
305+
.mockRejectedValueOnce(new Error('loader failed'))
306+
.mockResolvedValueOnce({ default: ping })
307+
308+
const rpcMatcher = new StandardRPCMatcher()
309+
rpcMatcher.init(base.router({
310+
ping: base.lazy(pingLoader),
311+
}))
312+
313+
await expect(rpcMatcher.match('POST', '/ping')).rejects.toThrow('loader failed')
314+
315+
expect(await rpcMatcher.match('POST', '/ping')).toEqual({
316+
path: ['ping'],
317+
procedure: ping,
318+
})
319+
320+
expect(pingLoader).toHaveBeenCalledTimes(2)
321+
})
322+
172323
it('filter procedures', async () => {
173324
const rpcMatcher = new StandardRPCMatcher({
174325
filter: (options) => {

0 commit comments

Comments
 (0)