diff --git a/dev-packages/e2e-tests/test-applications/astro-7/package.json b/dev-packages/e2e-tests/test-applications/astro-7/package.json index 92595307a056..33d76afc1eef 100644 --- a/dev-packages/e2e-tests/test-applications/astro-7/package.json +++ b/dev-packages/e2e-tests/test-applications/astro-7/package.json @@ -13,11 +13,11 @@ }, "//": "Need to use ioredis 5.10.1 because that's the last version before they support tracing channels", "dependencies": { - "@astrojs/node": "^11.0.0-alpha.0", + "@astrojs/node": "^11.1.4", "@playwright/test": "~1.56.0", "@sentry-internal/test-utils": "link:../../../test-utils", "@sentry/astro": "file:../../packed/sentry-astro-packed.tgz", - "astro": "beta", + "astro": "^7.2.4", "ioredis": "5.10.1", "mysql": "^2.18.1" }, diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 74fc38ff5612..00b3623aeb80 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -399,7 +399,8 @@ function checkIsDynamicPageRequest(context: APIContext): boolean { /** * Join Astro route segments into a case-sensitive single path string. * - * Astro lowercases the parametrized route. Joining segments manually is recommended to get the correct casing of the routes. + * Astro v5 and v6 lowercase the parametrized route. Joining segments manually + * is recommended to get the correct casing of the routes. * Recommendation in comment: https://github.com/withastro/astro/issues/13885#issuecomment-2934203029 * Function Reference: https://github.com/joanrieu/astro-typed-links/blob/b3dc12c6fe8d672a2bc2ae2ccc57c8071bbd09fa/package/src/integration.ts#L16 */ @@ -413,7 +414,7 @@ function joinRouteSegments(segments: RoutePart[][]): string { function getParametrizedRoute(ctx: APIContext & { routePattern?: string }): string | undefined { try { - // `routePattern` is available after Astro 5 + // `routePattern` is available from Astro 5 on. const contextWithRoutePattern = ctx; const rawRoutePattern = contextWithRoutePattern.routePattern; @@ -432,9 +433,12 @@ function getParametrizedRoute(ctx: APIContext & { routePattern?: string }): stri )?.routeData?.segments; return ( - // Astro v5+ - Joining the segments to get the correct casing of the parametrized route + // Astro v5 and v6 - Joining the segments to get the correct casing of the parametrized route (matchedRouteSegmentsFromManifest && joinRouteSegments(matchedRouteSegmentsFromManifest)) || - // Fallback (Astro v4 and earlier) + // Astro v7 - the manifest is no longer reachable from the context, but + // `routePattern` keeps the author's casing, so it needs no correction. + rawRoutePattern || + // Fallback (Astro v4 and earlier, which has no `routePattern`) interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params) ); } catch { diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 6bb15c58629f..83c87ca09a22 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -578,6 +578,107 @@ describe('sentryMiddleware', () => { }); }); +describe('parametrized route resolution', () => { + const startSpanSpy = vi.spyOn(SentryNode, 'startSpan'); + + beforeEach(() => { + vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation( + () => + ({ + setPropagationContext: vi.fn(), + getSpan: () => undefined, + setSDKProcessingMetadata: vi.fn(), + getPropagationContext: () => ({}), + }) as any, + ); + vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(() => undefined); + vi.spyOn(SentryNode, 'getClient').mockImplementation( + () => + ({ + getOptions: () => ({}), + getDataCollectionOptions: () => ({ httpHeaders: { request: false, response: false } }), + }) as unknown as Client, + ); + vi.spyOn(SentryNode, 'getTraceMetaTags').mockImplementation(() => ''); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + // Astro lowercases `routePattern`, so the manifest segments are the only way + // to recover the author's casing. Astro 7 dropped the lowercasing and, from + // 7.2.3, the manifest symbols too. + const MANIFEST_SEGMENTS = [ + [{ content: 'catchAll', dynamic: false, spread: false }], + [{ content: '...path', dynamic: true, spread: true }], + ]; + + function runMiddleware(ctx: Record): string | undefined { + const middleware = handleRequest(); + const next = vi.fn(() => Promise.resolve(new Response(null, { status: 200, headers: new Headers() }))); + // @ts-expect-error, a partial ctx object is fine here + middleware({ ...DYNAMIC_REQUEST_CONTEXT, ...ctx }, next); + return startSpanSpy.mock.lastCall?.[0]?.name; + } + + const CATCH_ALL_CTX = { + request: { method: 'GET', url: '/catchAll/a/b', headers: new Headers() }, + url: new URL('https://myDomain.io/catchAll/a/b'), + params: { path: 'a/b' }, + routePattern: '/catchAll/[...path]', + }; + + it.each([ + ['Astro 5', Symbol.for('context.routes')], + ['Astro 6', Symbol.for('astro.pipeline')], + ])('reads the route from the %s manifest, preserving casing', (_label, symbol) => { + const name = runMiddleware({ + ...CATCH_ALL_CTX, + // Astro lowercases `routePattern`, so this differs from the manifest casing. + routePattern: '/catchall/[...path]', + [symbol]: { + manifest: { routes: [{ routeData: { route: '/catchall/[...path]', segments: MANIFEST_SEGMENTS } }] }, + }, + }); + + expect(name).toBe('GET /catchAll/[...path]'); + }); + + // Astro 7.2.3 removed the last manifest symbol. `routePattern` keeps the + // author's casing there, so it is the correct source once the manifest is gone. + it('falls back to `routePattern` when no manifest is reachable', () => { + expect(runMiddleware(CATCH_ALL_CTX)).toBe('GET /catchAll/[...path]'); + }); + + it('prefers `routePattern` over interpolating a rest param out of the URL', () => { + // Interpolation reverse-maps param values found in the URL, so it cannot + // recover the `...` of a rest param. + expect(interpolateRouteFromUrlAndParams('/catchAll/a/b', { path: 'a/b' })).toBe('/catchAll/[path]'); + expect(runMiddleware(CATCH_ALL_CTX)).toBe('GET /catchAll/[...path]'); + }); + + it('falls back to `routePattern` when the manifest holds no matching route', () => { + const name = runMiddleware({ + ...CATCH_ALL_CTX, + [Symbol.for('astro.pipeline')]: { manifest: { routes: [{ routeData: { route: '/other', segments: [] } }] } }, + }); + + expect(name).toBe('GET /catchAll/[...path]'); + }); + + // Astro 4 has no `routePattern`, so interpolation stays the last resort. + it('interpolates from the URL when `routePattern` is absent', () => { + const name = runMiddleware({ + request: { method: 'GET', url: '/users/123/details', headers: new Headers() }, + url: new URL('https://myDomain.io/users/123/details'), + params: { id: '123' }, + }); + + expect(name).toBe('GET /users/[id]/details'); + }); +}); + describe('interpolateRouteFromUrlAndParams', () => { it.each([ ['/', {}, '/'],