Skip to content

Commit 2305d28

Browse files
committed
fix: handle routes without a handler and do not crash when processing or routes fail
1 parent 19c3a42 commit 2305d28

File tree

4 files changed

+127
-42
lines changed

4 files changed

+127
-42
lines changed

src/code_scanners/routes_scanner/main.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ export class RoutesScanner {
243243
*/
244244
#processRouteWithoutController(route: RoutesListItem) {
245245
if (!route.name) {
246-
debug(`skipping route "%s" as it does not have a name`, route.name)
246+
debug(`skipping route "%s" as it does not have a name`, route.pattern)
247247
return
248248
}
249249

@@ -275,7 +275,7 @@ export class RoutesScanner {
275275
* the case where someone imports the controllers and uses it by
276276
* reference
277277
*/
278-
if (!route.handler.importExpression) {
278+
if (!route.handler || !route.handler.importExpression) {
279279
return this.#processRouteWithoutController(route)
280280
}
281281

src/dev_server.ts

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,14 @@ export class DevServer {
454454
return
455455
}
456456

457-
const invalidated = await this.#routesScanner.invalidate(filePath)
458-
if (invalidated) {
459-
await this.#hooks.runner('routesScanned').run(this, this.#routesScanner)
457+
try {
458+
const invalidated = await this.#routesScanner.invalidate(filePath)
459+
if (invalidated) {
460+
await this.#hooks.runner('routesScanned').run(this, this.#routesScanner)
461+
}
462+
} catch (error) {
463+
this.ui.logger.error('Unable to rescan routes because of the following error')
464+
this.ui.logger.fatal(error)
460465
}
461466
}
462467

@@ -477,44 +482,49 @@ export class DevServer {
477482
* })
478483
*/
479484
#processRoutes = throttle(async (routesFileLocation: string) => {
480-
const scanRoutes = this.#hooks.has('routesScanning') || this.#hooks.has('routesScanned')
481-
const shareRoutes = this.#hooks.has('routesCommitted')
485+
try {
486+
const scanRoutes = this.#hooks.has('routesScanning') || this.#hooks.has('routesScanned')
487+
const shareRoutes = this.#hooks.has('routesCommitted')
482488

483-
/**
484-
* Remove the routes file and return early when there are no
485-
* hooks listening for routes related events
486-
*/
487-
if (!scanRoutes && !shareRoutes) {
489+
/**
490+
* Remove the routes file and return early when there are no
491+
* hooks listening for routes related events
492+
*/
493+
if (!scanRoutes && !shareRoutes) {
494+
unlink(routesFileLocation).catch(() => {})
495+
return
496+
}
497+
498+
/**
499+
* Read routes JSON, parse it and remove the file
500+
*/
501+
const routesJSON = await readFile(routesFileLocation, 'utf-8')
502+
const routesList = JSON.parse(routesJSON)
488503
unlink(routesFileLocation).catch(() => {})
489-
return
490-
}
491504

492-
/**
493-
* Read routes JSON, parse it and remove the file
494-
*/
495-
const routesJSON = await readFile(routesFileLocation, 'utf-8')
496-
const routesList = JSON.parse(routesJSON)
497-
unlink(routesFileLocation).catch(() => {})
505+
/**
506+
* Notify about the existence of routes
507+
*/
508+
if (shareRoutes) {
509+
await this.#hooks.runner('routesCommitted').run(this, routesList)
510+
}
498511

499-
/**
500-
* Notify about the existence of routes
501-
*/
502-
if (shareRoutes) {
503-
await this.#hooks.runner('routesCommitted').run(this, routesList)
504-
}
512+
/**
513+
* Scan routes and notify scanning and scanned hooks
514+
*/
515+
if (scanRoutes) {
516+
this.#routesScanner = new RoutesScanner(this.cwdPath, [])
517+
await this.#hooks.runner('routesScanning').run(this, this.#routesScanner)
505518

506-
/**
507-
* Scan routes and notify scanning and scanned hooks
508-
*/
509-
if (scanRoutes) {
510-
this.#routesScanner = new RoutesScanner(this.cwdPath, [])
511-
await this.#hooks.runner('routesScanning').run(this, this.#routesScanner)
519+
for (const domain of Object.keys(routesList)) {
520+
await this.#routesScanner.scan(routesList[domain])
521+
}
512522

513-
for (const domain of Object.keys(routesList)) {
514-
await this.#routesScanner.scan(routesList[domain])
523+
await this.#hooks.runner('routesScanned').run(this, this.#routesScanner)
515524
}
516-
517-
await this.#hooks.runner('routesScanned').run(this, this.#routesScanner)
525+
} catch (error) {
526+
this.ui.logger.error('Unable to process and scan routes because of the following error')
527+
this.ui.logger.fatal(error)
518528
}
519529
}, 'processRoutes')
520530

src/types/code_scanners.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,10 @@ export type RoutesListItem = {
189189
domain: string
190190
/** HTTP methods accepted by this route */
191191
methods: string[]
192-
handler:
193-
| Function
194-
| {
195-
method: string
196-
importExpression: string | null
197-
}
192+
handler?: {
193+
method: string
194+
importExpression: string | null
195+
}
198196
/** Parsed route tokens for URI construction */
199197
tokens: { val: string; old: string; type: 0 | 1 | 2 | 3; end: string }[]
200198
}

tests/code_scanners/routes_scanner.spec.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,4 +1424,81 @@ test.group('Routes scanner', () => {
14241424
]
14251425
`)
14261426
})
1427+
1428+
test('do not fail when route does not have a controller based handler', async ({
1429+
assert,
1430+
fs,
1431+
}) => {
1432+
await fs.createJson('package.json', {
1433+
imports: {
1434+
'#controllers/*': './app/controllers/*.ts',
1435+
},
1436+
})
1437+
1438+
const source = string.toUnixSlash(fs.basePath)
1439+
const scanner = new RoutesScanner(source, [])
1440+
scanner.pathsResolver.use((specifier) => {
1441+
const [namespace, ...rest] = specifier.split('/')
1442+
const fileName = rest.pop()
1443+
const resolvedPath = namespace === '#controllers' ? './app/controllers' : ''
1444+
return new URL([resolvedPath, ...rest, `${fileName}.ts`].join('/'), fs.baseUrl).href
1445+
})
1446+
1447+
await scanner.scan([
1448+
{
1449+
name: 'users.index',
1450+
domain: 'root',
1451+
methods: ['GET', 'HEAD'],
1452+
pattern: '/users',
1453+
tokens: [],
1454+
},
1455+
])
1456+
1457+
assert.deepEqual(scanner.getControllers(), [])
1458+
assert.snapshot(scanner.getScannedRoutes()).matchInline(`
1459+
[
1460+
{
1461+
"domain": "root",
1462+
"methods": [
1463+
"GET",
1464+
"HEAD",
1465+
],
1466+
"name": "users.index",
1467+
"pattern": "/users",
1468+
"request": undefined,
1469+
"response": undefined,
1470+
"tokens": [],
1471+
},
1472+
]
1473+
`)
1474+
})
1475+
1476+
test('skip route if it does not have a name', async ({ assert, fs }) => {
1477+
await fs.createJson('package.json', {
1478+
imports: {
1479+
'#controllers/*': './app/controllers/*.ts',
1480+
},
1481+
})
1482+
1483+
const source = string.toUnixSlash(fs.basePath)
1484+
const scanner = new RoutesScanner(source, [])
1485+
scanner.pathsResolver.use((specifier) => {
1486+
const [namespace, ...rest] = specifier.split('/')
1487+
const fileName = rest.pop()
1488+
const resolvedPath = namespace === '#controllers' ? './app/controllers' : ''
1489+
return new URL([resolvedPath, ...rest, `${fileName}.ts`].join('/'), fs.baseUrl).href
1490+
})
1491+
1492+
await scanner.scan([
1493+
{
1494+
domain: 'root',
1495+
methods: ['GET', 'HEAD'],
1496+
pattern: '/users',
1497+
tokens: [],
1498+
},
1499+
])
1500+
1501+
assert.deepEqual(scanner.getControllers(), [])
1502+
assert.snapshot(scanner.getScannedRoutes()).matchInline('[]')
1503+
})
14271504
})

0 commit comments

Comments
 (0)