Skip to content

Commit 942a721

Browse files
committed
fix(routing): improve error handling
1 parent 09b533b commit 942a721

File tree

4 files changed

+107
-1
lines changed

4 files changed

+107
-1
lines changed

.changeset/witty-wasps-tie.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Improved the error message when a redirect can't be mapped to its destination. For example, the following redirect
6+
will throw a new error because `/category/[categories]/` contains one dynamic segment, while `/category/[categories]/[page]` has two dynamic segments,
7+
and Astro doesn't know how map the parameters:
8+
9+
```js
10+
export default defineConfig({
11+
"/category/[categories]": "/category/[categories]/[page]"
12+
})
13+
```

packages/astro/src/core/errors/errors-data.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,26 @@ export const UnsupportedExternalRedirect = {
981981
hint: 'An external redirect must start with http or https, and must be a valid URL.',
982982
} satisfies ErrorData;
983983

984+
/**
985+
* @docs
986+
* @see
987+
* - [Astro.redirect](https://docs.astro.build/en/reference/api-reference/#redirect)
988+
* @description
989+
* A redirect can't be mapped when origin and destination have different segments.
990+
*/
991+
export const RedirectMappingMismatch = {
992+
name: 'RedirectMappingMismatch',
993+
title: "A redirect can't be mapped when origin and destination have different segments",
994+
message(
995+
fromRoute: string,
996+
toRoute: string,
997+
fromDynamicSegments: number,
998+
toDynamicSegments: number,
999+
) {
1000+
return `The number of dynamic segments don't match. The route ${fromRoute} has ${fromDynamicSegments} segments, while ${toRoute} has ${toDynamicSegments} segments.`;
1001+
},
1002+
} satisfies ErrorData;
1003+
9841004
/**
9851005
* @docs
9861006
* @see

packages/astro/src/core/routing/manifest/create.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type { RouteData, RoutePart } from '../../../types/public/internal.js';
1414
import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js';
1515
import {
1616
MissingIndexForInternationalization,
17+
RedirectMappingMismatch,
1718
UnsupportedExternalRedirect,
1819
} from '../../errors/errors-data.js';
1920
import { AstroError } from '../../errors/index.js';
@@ -364,6 +365,12 @@ function createRedirectRoutes(
364365
});
365366
}
366367

368+
const redirectRoute = routeMap.get(destination);
369+
370+
if (redirectRoute) {
371+
validateRouteMapping(segments, from, redirectRoute.segments ?? [], redirectRoute.route ?? '');
372+
}
373+
367374
routes.push({
368375
type: 'redirect',
369376
// For backwards compatibility, a redirect is never considered an index route.
@@ -377,7 +384,7 @@ function createRedirectRoutes(
377384
pathname: pathname || void 0,
378385
prerender: getPrerenderDefault(config),
379386
redirect: to,
380-
redirectRoute: routeMap.get(destination),
387+
redirectRoute,
381388
fallbackRoutes: [],
382389
distURL: [],
383390
origin: 'project',
@@ -387,6 +394,54 @@ function createRedirectRoutes(
387394
return routes;
388395
}
389396

397+
/**
398+
* Checks whether `a` can be mapped to `b`. If the requirements don't match,
399+
* an error is thrown.
400+
* @param fromSegments
401+
* @param fromRoute
402+
* @param toSegments
403+
* @param toRoute
404+
*/
405+
function validateRouteMapping(
406+
fromSegments: RoutePart[][],
407+
fromRoute: string,
408+
toSegments: RoutePart[][],
409+
toRoute: string,
410+
) {
411+
const fromHasSpread = fromSegments.some((routePart) => routePart.some((part) => part.spread));
412+
const toHasSpread = toSegments.some((routePart) => routePart.some((part) => part.spread));
413+
414+
if (fromHasSpread || toHasSpread) {
415+
return;
416+
}
417+
418+
const fromDynamicSegments = fromSegments.reduce((dynamicSegments, routePart) => {
419+
if (routePart.some((part) => part.dynamic || part.spread)) {
420+
dynamicSegments += 1;
421+
}
422+
return dynamicSegments;
423+
}, 0);
424+
425+
const toDynamicSegments = toSegments.reduce((dynamicSegments, routePart) => {
426+
if (routePart.some((part) => part.dynamic || part.spread)) {
427+
dynamicSegments += 1;
428+
}
429+
return dynamicSegments;
430+
}, 0);
431+
432+
if (fromDynamicSegments != toDynamicSegments) {
433+
throw new AstroError({
434+
...RedirectMappingMismatch,
435+
message: RedirectMappingMismatch.message(
436+
fromRoute,
437+
toRoute,
438+
fromDynamicSegments,
439+
toDynamicSegments,
440+
),
441+
});
442+
}
443+
}
444+
390445
/**
391446
* Checks whether a route segment is static.
392447
*/

packages/astro/test/redirects.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,4 +345,22 @@ describe('Astro.redirect', () => {
345345
assert.equal(secretHtml.includes('url=/login'), true);
346346
});
347347
});
348+
349+
describe('should fail for redirects that cannot be mapped ', () => {
350+
it('should fail for redirects that cannot be mapped', async () => {
351+
fixture = await loadFixture({
352+
root: './fixtures/redirects/',
353+
output: 'static',
354+
redirects: {
355+
'/old/[category]/': '/old/[category]/[slug]',
356+
},
357+
});
358+
try {
359+
await fixture.build();
360+
assert.fail('Expected build to fail');
361+
} catch {
362+
assert.ok(true);
363+
}
364+
});
365+
});
348366
});

0 commit comments

Comments
 (0)