-
Notifications
You must be signed in to change notification settings - Fork 6
fix: resolve async factory services in dispatcher and all framework adapters #859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b9a5fa4
8140bc3
b1b6e99
eda02c8
77086c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -81,10 +81,18 @@ export function createExpressRouter(options: ExpressAdapterOptions): Router { | |||||||||||||||||||||||||||||||||||||||||||||||||
| const path = (req.params as any).path; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const method = req.method; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const authService = typeof options.kernel.getService === 'function' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ? options.kernel.getService<AuthService>('auth') | ||||||||||||||||||||||||||||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| let authService: AuthService | null = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof options.kernel.getServiceAsync === 'function') { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| authService = await options.kernel.getServiceAsync<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (typeof options.kernel.getService === 'function') { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| authService = options.kernel.getService<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Service not registered — fall through to dispatcher | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+92
to
+93
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | |
| // Service not registered — fall through to dispatcher | |
| } catch (err) { | |
| const message = | |
| err instanceof Error | |
| ? err.message | |
| : typeof err === 'string' | |
| ? err | |
| : String(err); | |
| const isNotRegistered = | |
| message.includes('not registered') || | |
| message.includes('not found') || | |
| message.includes('service "auth"'); | |
| const isAsyncHint = | |
| message.includes('is async - use await') || | |
| message.includes('use getServiceAsync'); | |
| if (!isNotRegistered && !isAsyncHint) { | |
| // Unexpected error during auth service resolution — surface as 500 | |
| throw err; | |
| } | |
| // Service not registered / not available — fall through to dispatcher |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,10 +84,18 @@ export async function objectStackPlugin(fastify: FastifyInstance, options: Fasti | |
| const path = request.url.substring(`${prefix}/auth/`.length).split('?')[0]; | ||
| const method = request.method; | ||
|
|
||
| // Try AuthPlugin service first | ||
| const authService = typeof options.kernel.getService === 'function' | ||
| ? options.kernel.getService<AuthService>('auth') | ||
| : null; | ||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||
| let authService: AuthService | null = null; | ||
| try { | ||
| if (typeof options.kernel.getServiceAsync === 'function') { | ||
| authService = await options.kernel.getServiceAsync<AuthService>('auth'); | ||
| } else if (typeof options.kernel.getService === 'function') { | ||
| authService = options.kernel.getService<AuthService>('auth'); | ||
| } | ||
|
Comment on lines
+90
to
+94
|
||
| } catch { | ||
| // Service not registered — fall through to dispatcher | ||
| authService = null; | ||
| } | ||
|
Comment on lines
+95
to
+98
|
||
|
|
||
| if (authService && typeof authService.handleRequest === 'function') { | ||
| const protocol = request.protocol || 'http'; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -87,10 +87,18 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { | |||||||||||||||||||||||||||||||||||||||||||||
| const path = c.req.path.substring(`${prefix}/auth/`.length); | ||||||||||||||||||||||||||||||||||||||||||||||
| const method = c.req.method; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (preferred path) | ||||||||||||||||||||||||||||||||||||||||||||||
| const authService = typeof options.kernel.getService === 'function' | ||||||||||||||||||||||||||||||||||||||||||||||
| ? options.kernel.getService<AuthService>('auth') | ||||||||||||||||||||||||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||||||||||||||||||||||||||||||||||||||||||||||
| let authService: AuthService | null = null; | ||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof options.kernel.getServiceAsync === 'function') { | ||||||||||||||||||||||||||||||||||||||||||||||
| authService = await options.kernel.getServiceAsync<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||||||||||||
| } else if (typeof options.kernel.getService === 'function') { | ||||||||||||||||||||||||||||||||||||||||||||||
| authService = options.kernel.getService<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | |
| } catch (err: any) { | |
| // Only treat known "service not registered / async lookup" errors as absence; | |
| // rethrow unexpected errors so they surface as 500s via the outer handler. | |
| const message = | |
| err && typeof err.message === 'string' ? err.message.toLowerCase() : ''; | |
| const isLookupError = | |
| err?.code === 'SERVICE_NOT_FOUND' || | |
| err?.code === 'SERVICE_NOT_REGISTERED' || | |
| err?.name === 'ServiceNotFoundError' || | |
| message.includes('service not found') || | |
| message.includes('service "auth" not found') || | |
| message.includes('service auth not found') || | |
| message.includes('service not registered') || | |
| message.includes('no auth service registered') || | |
| message.includes('service lookup is async') || | |
| message.includes('use getserviceasync') || | |
| message.includes('is async'); | |
| if (!isLookupError) { | |
| throw err; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -113,11 +113,19 @@ export class ObjectStackController { | |||||||||||||||||||||||||||||||||||
| @All('auth/*') | ||||||||||||||||||||||||||||||||||||
| async auth(@Req() req: any, @Res() res: any, @Body() body: any) { | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (preferred path) | ||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||||||||||||||||||||||||||||||||||||
| const kernel = this.service.getKernel(); | ||||||||||||||||||||||||||||||||||||
| const authService = typeof kernel.getService === 'function' | ||||||||||||||||||||||||||||||||||||
| ? kernel.getService<AuthService>('auth') | ||||||||||||||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||||||||||||||
| let authService: AuthService | null = null; | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| if (typeof kernel.getServiceAsync === 'function') { | ||||||||||||||||||||||||||||||||||||
| authService = await kernel.getServiceAsync<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||
| } else if (typeof kernel.getService === 'function') { | ||||||||||||||||||||||||||||||||||||
| authService = kernel.getService<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+120
to
+124
|
||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||
| // Service not registered — fall through to legacy dispatcher | ||||||||||||||||||||||||||||||||||||
| authService = null; | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+125
to
+127
|
||||||||||||||||||||||||||||||||||||
| } catch { | |
| // Service not registered — fall through to legacy dispatcher | |
| authService = null; | |
| } catch (error: any) { | |
| const message = typeof error?.message === 'string' ? error.message : ''; | |
| const isNotRegistered = | |
| message.includes('not registered') || message.includes('not found'); | |
| const isAsyncHint = message.includes('is async'); | |
| if (isNotRegistered || isAsyncHint) { | |
| // Auth service not registered or only available asynchronously — | |
| // fall through to legacy dispatcher. | |
| authService = null; | |
| } else { | |
| // Unexpected error during auth service resolution — surface it. | |
| throw error; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -68,10 +68,18 @@ export function createRouteHandler(options: NextAdapterOptions) { | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // --- 1. Auth --- | ||||||||||||||||||||||||||||||||||||
| if (segments[0] === 'auth') { | ||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (preferred path) | ||||||||||||||||||||||||||||||||||||
| const authService = typeof options.kernel.getService === 'function' | ||||||||||||||||||||||||||||||||||||
| ? options.kernel.getService<AuthService>('auth') | ||||||||||||||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||||||||||||||||||||||||||||||||||||
| let authService: AuthService | null = null; | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| if (typeof options.kernel.getServiceAsync === 'function') { | ||||||||||||||||||||||||||||||||||||
| authService = await options.kernel.getServiceAsync<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||
| } else if (typeof options.kernel.getService === 'function') { | ||||||||||||||||||||||||||||||||||||
| authService = options.kernel.getService<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||
| // Service not registered — fall through to dispatcher | ||||||||||||||||||||||||||||||||||||
| authService = null; | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+79
to
+81
|
||||||||||||||||||||||||||||||||||||
| } catch { | |
| // Service not registered — fall through to dispatcher | |
| authService = null; | |
| } catch (err) { | |
| // Only treat expected "service not registered" lookup errors as absence. | |
| const isLookupError = | |
| err instanceof Error && | |
| typeof err.message === 'string' && | |
| /not\s+(registered|found)/i.test(err.message); | |
| if (isLookupError) { | |
| // Service not registered — fall through to dispatcher | |
| authService = null; | |
| } else { | |
| // Unexpected error during auth service resolution: rethrow so it remains observable | |
| throw err; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -104,10 +104,18 @@ export function createH3Router(options: NuxtAdapterOptions): Router { | |||||||||||||||||||||||||||||||||||||
| const path = urlPath.substring(`${prefix}/auth/`.length).split('?')[0]; | ||||||||||||||||||||||||||||||||||||||
| const method = event.method; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first | ||||||||||||||||||||||||||||||||||||||
| const authService = typeof options.kernel.getService === 'function' | ||||||||||||||||||||||||||||||||||||||
| ? options.kernel.getService<AuthService>('auth') | ||||||||||||||||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||||||||||||||||||||||||||||||||||||||
| let authService: AuthService | null = null; | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| if (typeof options.kernel.getServiceAsync === 'function') { | ||||||||||||||||||||||||||||||||||||||
| authService = await options.kernel.getServiceAsync<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||||
| } else if (typeof options.kernel.getService === 'function') { | ||||||||||||||||||||||||||||||||||||||
| authService = options.kernel.getService<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+110
to
+114
|
||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||
| // Service not registered — fall through to dispatcher | ||||||||||||||||||||||||||||||||||||||
| authService = null; | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+115
to
+117
|
||||||||||||||||||||||||||||||||||||||
| } catch { | |
| // Service not registered — fall through to dispatcher | |
| authService = null; | |
| } catch (err: any) { | |
| const message = typeof err?.message === 'string' ? err.message : ''; | |
| const lowerMessage = message.toLowerCase(); | |
| const isLookupError = | |
| lowerMessage.includes('not registered') || | |
| lowerMessage.includes('not found') || | |
| lowerMessage.includes('is async'); | |
| if (isLookupError) { | |
| // Service not registered or only available asynchronously — fall through to dispatcher | |
| authService = null; | |
| } else { | |
| // Unexpected error — rethrow so it surfaces as a 500 | |
| throw err; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -108,10 +108,18 @@ export function createRequestHandler(options: SvelteKitAdapterOptions) { | |||||||||||||||||||||||||||||||||
| if (segments[0] === 'auth') { | ||||||||||||||||||||||||||||||||||
| const subPath = segments.slice(1).join('/'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first | ||||||||||||||||||||||||||||||||||
| const authService = typeof options.kernel.getService === 'function' | ||||||||||||||||||||||||||||||||||
| ? options.kernel.getService<AuthService>('auth') | ||||||||||||||||||||||||||||||||||
| : null; | ||||||||||||||||||||||||||||||||||
| // Try AuthPlugin service first (prefer async to support factory-based services) | ||||||||||||||||||||||||||||||||||
| let authService: AuthService | null = null; | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| if (typeof options.kernel.getServiceAsync === 'function') { | ||||||||||||||||||||||||||||||||||
| authService = await options.kernel.getServiceAsync<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||
| } else if (typeof options.kernel.getService === 'function') { | ||||||||||||||||||||||||||||||||||
| authService = options.kernel.getService<AuthService>('auth'); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||
| // Service not registered — fall through to dispatcher | ||||||||||||||||||||||||||||||||||
| authService = null; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+119
to
+121
|
||||||||||||||||||||||||||||||||||
| } catch { | |
| // Service not registered — fall through to dispatcher | |
| authService = null; | |
| } catch (error) { | |
| // Only swallow expected lookup errors; rethrow unexpected failures | |
| if (error instanceof Error) { | |
| const message = error.message || ''; | |
| if (message.includes('not registered') || message.includes('is async')) { | |
| // Service not registered or requires async access — fall through to dispatcher | |
| authService = null; | |
| } else { | |
| throw error; | |
| } | |
| } else { | |
| throw error; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Express adapter changed auth resolution to prefer kernel.getServiceAsync(), but the adapter test suite doesn’t cover this branch. Add a test that supplies getServiceAsync('auth') and asserts it is used (and another that asserts fallthrough when getServiceAsync throws/returns null) to prevent regressions.