Skip to content

Commit b0ed93d

Browse files
committed
perf(router): resolve cached middleware without async wrappers
1 parent fb53f7d commit b0ed93d

3 files changed

Lines changed: 62 additions & 7 deletions

File tree

storage/framework/core/router/src/stacks-router.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,13 +1283,16 @@ function negateMiddleware(name: string, inner: MiddlewareHandler): MiddlewareHan
12831283
/**
12841284
* Load the handler a parsed reference names, negated when it asked to be.
12851285
*/
1286-
async function loadParsedMiddleware(parsed: ParsedMiddleware): Promise<MiddlewareHandler | null> {
1287-
const handler = await loadMiddleware(parsed.name)
1288-
1289-
if (!handler || !parsed.negated)
1290-
return handler
1286+
function loadParsedMiddleware(parsed: ParsedMiddleware): MiddlewareHandler | null | Promise<MiddlewareHandler | null> {
1287+
// Resolved modules are synchronous values. Keep cached failures as null so
1288+
// the caller still fails closed, and use the same cache hot reload clears.
1289+
const cached = middlewareCache.get(parsed.name)
1290+
if (cached !== undefined)
1291+
return cached && parsed.negated ? negateMiddleware(parsed.name, cached) : cached
12911292

1292-
return negateMiddleware(parsed.name, handler)
1293+
return loadMiddleware(parsed.name).then(handler =>
1294+
handler && parsed.negated ? negateMiddleware(parsed.name, handler) : handler,
1295+
)
12931296
}
12941297

12951298
/**
@@ -1699,7 +1702,8 @@ function createMiddlewareHandler(routeKey: string, handler: StacksHandler): Rout
16991702
;enhancedReq._middlewareParams[middlewareName] = params
17001703
}
17011704

1702-
const middleware = await loadParsedMiddleware(parsed)
1705+
const loaded = loadParsedMiddleware(parsed)
1706+
const middleware = loaded instanceof Promise ? await loaded : loaded
17031707
if (!middleware || typeof middleware.handle !== 'function') {
17041708
// Fail CLOSED. The previous `continue` served the route WITHOUT
17051709
// the middleware — a typo'd `auth` alias silently unprotected the

storage/framework/core/router/tests/middleware-fail-closed.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
*/
2121

2222
import { afterAll, beforeEach, describe, expect, test } from 'bun:test'
23+
import { unlinkSync, writeFileSync } from 'node:fs'
24+
import { appPath } from '@stacksjs/path'
2325
import { assertRouteMiddlewareResolvable, clearMiddlewareCache, clearRouteMiddlewareRegistry, createStacksRouter, findUnresolvableRouteMiddleware } from '../src/stacks-router'
2426

2527
beforeEach(() => {
@@ -37,6 +39,31 @@ afterAll(() => {
3739
})
3840

3941
describe('request-time fail-closed', () => {
42+
test('a broken middleware stays closed when its failed load is cached', async () => {
43+
const name = `BrokenCached${crypto.randomUUID().replaceAll('-', '')}`
44+
const file = appPath(`Middleware/${name}.ts`)
45+
writeFileSync(file, 'export default {}\n')
46+
let handlerRuns = 0
47+
48+
try {
49+
const router = createStacksRouter()
50+
router.get('/mw-broken-cached', () => {
51+
handlerRuns++
52+
return { ok: true }
53+
}).middleware(name)
54+
55+
for (let request = 0; request < 2; request++) {
56+
const res = await router.handleRequest(new Request('http://localhost/mw-broken-cached'))
57+
expect(res.status).toBe(500)
58+
expect(handlerRuns).toBe(0)
59+
}
60+
}
61+
finally {
62+
unlinkSync(file)
63+
clearMiddlewareCache()
64+
}
65+
})
66+
4067
test('resolvable alias → middleware runs, handler runs, 200', async () => {
4168
const router = createStacksRouter()
4269
let handlerRan = false

storage/framework/core/router/tests/middleware-references.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,30 @@ describe('alias lookup happens before the colon is read as a parameter', () => {
8686
})
8787

8888
describe('negated references', () => {
89+
test('warm handlers and reloaded handlers enforce the current request environment', async () => {
90+
const previous = config.app.env
91+
const router = createStacksRouter()
92+
router.get('/mw-cache-prod', () => ({ ok: true })).middleware('env:production')
93+
router.get('/mw-cache-not-prod', () => ({ ok: true })).middleware('!env:production')
94+
95+
try {
96+
for (let cycle = 0; cycle < 2; cycle++) {
97+
clearMiddlewareCache()
98+
for (const env of ['production', 'local', 'production']) {
99+
config.app.env = env as typeof config.app.env
100+
const allowed = await router.handleRequest(new Request('http://localhost/mw-cache-prod'))
101+
const negated = await router.handleRequest(new Request('http://localhost/mw-cache-not-prod'))
102+
expect(allowed.status).toBe(env === 'production' ? 200 : 403)
103+
expect(negated.status).toBe(env === 'production' ? 403 : 200)
104+
}
105+
}
106+
}
107+
finally {
108+
config.app.env = previous
109+
clearMiddlewareCache()
110+
}
111+
})
112+
89113
test('!auth passes when auth refuses', async () => {
90114
const router = createStacksRouter()
91115
let handlerRan = false

0 commit comments

Comments
 (0)