Skip to content

Commit cb6da8e

Browse files
authored
fix(hono): Use generic Hono type in Bun/Node (#21060)
1 parent 84f91cc commit cb6da8e

7 files changed

Lines changed: 202 additions & 6 deletions

File tree

dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import type { Hono } from 'hono';
1+
import { type Hono as HonoType, Hono } from 'hono';
22
import { HTTPException } from 'hono/http-exception';
33
import { failingMiddleware, middlewareA, middlewareB } from './middleware';
44
import { errorRoutes } from './route-groups/test-errors';
55
import { middlewareRoutes, subAppWithInlineMiddleware, subAppWithMiddleware } from './route-groups/test-middleware';
66
import { multiFetchRoutes } from './route-groups/test-multi-fetch';
77
import { routePatterns } from './route-groups/test-route-patterns';
88

9-
export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): void {
9+
export function addRoutes(app: HonoType<{ Bindings?: { E2E_TEST_DSN: string } }>): void {
1010
app.get('/', c => {
1111
return c.text('Hello Hono!');
1212
});
@@ -52,4 +52,27 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v
5252

5353
// Multi-fetch routes: storefront sub-app calls inventoryApp via .request()
5454
app.route('/test-multi-fetch', multiFetchRoutes);
55+
56+
// .basePath() with sub-app mounting via .route()
57+
const apiSubApp = new Hono();
58+
apiSubApp.use(async function apiAuth(_c, next) {
59+
await next();
60+
});
61+
apiSubApp.get('/users', c => c.json({ users: [{ id: 1, name: 'Alice' }] }));
62+
apiSubApp.get('/users/:userId', c => c.json({ userId: c.req.param('userId') }));
63+
64+
app.basePath('/test-basepath').route('/v1', apiSubApp);
65+
66+
// .use() on the cloned instance returned by .basePath() — the clone has its own
67+
// .use class field, so this tests whether middleware instrumentation propagates.
68+
app
69+
.basePath('/test-basepath-mw')
70+
.use(async function basepathMiddleware(_c, next) {
71+
await new Promise(resolve => setTimeout(resolve, 50));
72+
await next();
73+
})
74+
.get('/hello', c => c.json({ greeting: 'world' }));
75+
76+
// .get() registered on the root app after .basePath()/.route() chains
77+
app.get('/test-late-get', c => c.json({ registered: 'after-chains' }));
5578
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForTransaction } from '@sentry-internal/test-utils';
3+
import { APP_NAME } from './constants';
4+
5+
test.describe('basePath with sub-app routes', () => {
6+
test('traces GET on a sub-app mounted via .basePath().route()', async ({ baseURL }) => {
7+
const transactionPromise = waitForTransaction(APP_NAME, event => {
8+
return event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-basepath/v1/users';
9+
});
10+
11+
const response = await fetch(`${baseURL}/test-basepath/v1/users`);
12+
expect(response.status).toBe(200);
13+
14+
const body = await response.json();
15+
expect(body).toEqual({ users: [{ id: 1, name: 'Alice' }] });
16+
17+
const transaction = await transactionPromise;
18+
expect(transaction.transaction).toBe('GET /test-basepath/v1/users');
19+
expect(transaction.contexts?.trace?.op).toBe('http.server');
20+
});
21+
22+
test('traces parameterized route under .basePath().route()', async ({ baseURL }) => {
23+
const transactionPromise = waitForTransaction(APP_NAME, event => {
24+
return event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-basepath/v1/users/:userId';
25+
});
26+
27+
const response = await fetch(`${baseURL}/test-basepath/v1/users/42`);
28+
expect(response.status).toBe(200);
29+
30+
const body = await response.json();
31+
expect(body).toEqual({ userId: '42' });
32+
33+
const transaction = await transactionPromise;
34+
expect(transaction.transaction).toBe('GET /test-basepath/v1/users/:userId');
35+
expect(transaction.contexts?.trace?.op).toBe('http.server');
36+
});
37+
});
38+
39+
// TODO: this test is currently skipped because we do not yet support middleware registered on new instances (e.g. here via .basePath(..).use(...)).
40+
test.skip('.basePath() middleware instrumentation', () => {
41+
test('creates middleware span for .use() on .basePath() clone', async ({ baseURL }) => {
42+
const transactionPromise = waitForTransaction(APP_NAME, event => {
43+
return event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-basepath-mw/hello';
44+
});
45+
46+
const response = await fetch(`${baseURL}/test-basepath-mw/hello`);
47+
expect(response.status).toBe(200);
48+
49+
const body = await response.json();
50+
expect(body).toEqual({ greeting: 'world' });
51+
52+
const transaction = await transactionPromise;
53+
expect(transaction.transaction).toBe('GET /test-basepath-mw/hello');
54+
55+
const spans = transaction.spans || [];
56+
const middlewareSpan = spans.find(
57+
(span: { description?: string; op?: string }) =>
58+
span.op === 'middleware.hono' && span.description === 'basepathMiddleware',
59+
);
60+
61+
expect(middlewareSpan).toBeDefined();
62+
expect(middlewareSpan?.origin).toBe('auto.middleware.hono');
63+
});
64+
});
65+
66+
test('traces .get() route registered after .basePath()/.route() chains', async ({ baseURL }) => {
67+
const transactionPromise = waitForTransaction(APP_NAME, event => {
68+
return event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-late-get';
69+
});
70+
71+
const response = await fetch(`${baseURL}/test-late-get`);
72+
expect(response.status).toBe(200);
73+
74+
const body = await response.json();
75+
expect(body).toEqual({ registered: 'after-chains' });
76+
77+
const transaction = await transactionPromise;
78+
expect(transaction.transaction).toBe('GET /test-late-get');
79+
expect(transaction.contexts?.trace?.op).toBe('http.server');
80+
});

packages/hono/src/bun/middleware.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { type BaseTransportOptions, debug, type Options } from '@sentry/core';
22
import { init } from './sdk';
3-
import type { Hono, MiddlewareHandler } from 'hono';
3+
import type { Env, Hono, MiddlewareHandler } from 'hono';
44
import { requestHandler, responseHandler } from '../shared/middlewareHandlers';
55
import { applyPatches } from '../shared/applyPatches';
66

@@ -9,7 +9,7 @@ export interface HonoBunOptions extends Options<BaseTransportOptions> {}
99
/**
1010
* Sentry middleware for Hono running in a Bun runtime environment.
1111
*/
12-
export const sentry = (app: Hono, options: HonoBunOptions): MiddlewareHandler => {
12+
export const sentry = <E extends Env>(app: Hono<E>, options: HonoBunOptions): MiddlewareHandler => {
1313
const isDebug = options.debug;
1414

1515
isDebug && debug.log('Initialized Sentry Hono middleware (Bun)');

packages/hono/src/node/middleware.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { type BaseTransportOptions, debug, type Options, getClient } from '@sentry/core';
2-
import type { Hono, MiddlewareHandler } from 'hono';
2+
import type { Env, Hono, MiddlewareHandler } from 'hono';
33
import { requestHandler, responseHandler } from '../shared/middlewareHandlers';
44
import { applyPatches } from '../shared/applyPatches';
55

@@ -13,7 +13,7 @@ export interface HonoNodeOptions extends Options<BaseTransportOptions> {}
1313
*
1414
* **Note:** You must initialize Sentry separately before using this middleware. Typically, this is done by calling `Sentry.init()` in an `instrument.ts` file and loading it via the Node `--import` flag.
1515
*/
16-
export const sentry = (app: Hono): MiddlewareHandler => {
16+
export const sentry = <E extends Env>(app: Hono<E>): MiddlewareHandler => {
1717
const sentryClient = getClient();
1818
if (sentryClient === undefined) {
1919
debug.warn(

packages/hono/test/bun/middleware.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ describe('Hono Bun Middleware', () => {
2929
});
3030

3131
describe('sentry middleware', () => {
32+
it('accepts Hono with custom env types without requiring a cast', () => {
33+
type CustomEnv = { Bindings: { DATABASE_URL: string }; Variables: { userId: string } };
34+
const app = new Hono<CustomEnv>();
35+
36+
const middleware = sentry(app, { dsn: 'https://public@dsn.ingest.sentry.io/1337' });
37+
38+
expect(typeof middleware).toBe('function');
39+
expect(middleware).toHaveLength(2);
40+
});
41+
3242
it('calls applySdkMetadata with "hono" and "bun"', () => {
3343
const app = new Hono();
3444
const options = {

packages/hono/test/node/middleware.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ describe('Hono Node Middleware', () => {
3636
expect(initNodeMock).not.toHaveBeenCalled();
3737
});
3838

39+
it('accepts Hono with custom env types without requiring a cast', () => {
40+
type CustomEnv = { Bindings: { DATABASE_URL: string }; Variables: { userId: string } };
41+
const app = new Hono<CustomEnv>();
42+
43+
const middleware = sentry(app);
44+
45+
expect(typeof middleware).toBe('function');
46+
expect(middleware).toHaveLength(2);
47+
});
48+
3949
it('returns a middleware handler function', () => {
4050
const app = new Hono();
4151
const middleware = sentry(app);

packages/hono/test/shared/applyPatches.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,79 @@ describe('applyPatches', () => {
494494
});
495495
});
496496

497+
describe('main-app .get() routes after applyPatches', () => {
498+
it('responds correctly from .get() routes registered after applyPatches', async () => {
499+
const app = new Hono();
500+
applyPatches(app);
501+
502+
app.get('/docs', c => c.text('API Documentation'));
503+
app.get('/openapi.json', c => c.json({ openapi: '3.0.0', paths: {} }));
504+
505+
const docsRes = await app.fetch(new Request('http://localhost/docs'));
506+
expect(docsRes.status).toBe(200);
507+
expect(await docsRes.text()).toBe('API Documentation');
508+
509+
const specRes = await app.fetch(new Request('http://localhost/openapi.json'));
510+
expect(specRes.status).toBe(200);
511+
expect(await specRes.json()).toEqual({ openapi: '3.0.0', paths: {} });
512+
});
513+
514+
it('preserves .get() routes registered after .basePath() and .route() chains', async () => {
515+
const app = new Hono();
516+
applyPatches(app);
517+
518+
const subApp = new Hono();
519+
subApp.use(async function authMiddleware(_c: unknown, next: () => Promise<void>) {
520+
await next();
521+
});
522+
subApp.get('/resource', () => new Response('resource'));
523+
524+
app.basePath('/api').route('/v1', subApp);
525+
526+
app.get('/docs', c => c.text('Docs page'));
527+
app.get('/openapi.json', c => c.json({ openapi: '3.0.0' }));
528+
529+
const resourceRes = await app.fetch(new Request('http://localhost/api/v1/resource'));
530+
expect(resourceRes.status).toBe(200);
531+
expect(await resourceRes.text()).toBe('resource');
532+
533+
const docsRes = await app.fetch(new Request('http://localhost/docs'));
534+
expect(docsRes.status).toBe(200);
535+
expect(await docsRes.text()).toBe('Docs page');
536+
537+
const specRes = await app.fetch(new Request('http://localhost/openapi.json'));
538+
expect(specRes.status).toBe(200);
539+
expect(await specRes.json()).toEqual({ openapi: '3.0.0' });
540+
});
541+
542+
it('does not corrupt app.routes for third-party route introspection', () => {
543+
const app = new Hono();
544+
applyPatches(app);
545+
546+
app.use(async function globalMw(_c: unknown, next: () => Promise<void>) {
547+
await next();
548+
});
549+
app.get('/users', () => new Response('users'));
550+
app.post('/users', () => new Response('created'));
551+
552+
const subApp = new Hono();
553+
subApp.get('/items', () => new Response('items'));
554+
app.route('/api', subApp);
555+
556+
const routes = app.routes as Array<{ method: string; path: string; handler: Function }>;
557+
const getPaths = routes.filter(r => r.method === 'GET').map(r => r.path);
558+
const postPaths = routes.filter(r => r.method === 'POST').map(r => r.path);
559+
560+
expect(getPaths).toContain('/users');
561+
expect(getPaths).toContain('/api/items');
562+
expect(postPaths).toContain('/users');
563+
564+
for (const route of routes) {
565+
expect(typeof route.handler).toBe('function');
566+
}
567+
});
568+
});
569+
497570
describe('patchAppRequest integration', () => {
498571
it('patches .request() on sub-apps when they are mounted via route()', async () => {
499572
const app = new Hono();

0 commit comments

Comments
 (0)