Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ app.use(
sentry(app, {
dsn: process.env.SENTRY_DSN,
tracesSampleRate: 1.0,
debug: true,
// fixme - check out what removing this integration changes
// integrations: integrations => integrations.filter(integration => integration.name !== 'Hono'),
}),
);

Expand All @@ -26,7 +23,7 @@ app.get('/json', c => {
return c.json({ message: 'Hello from Hono', framework: 'hono', platform: 'cloudflare' });
});

app.get('/error', () => {
app.get('/error/:param', () => {
throw new Error('Test error from Hono app');
});

Expand Down
38 changes: 31 additions & 7 deletions dev-packages/cloudflare-integration-tests/suites/hono-sdk/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import { expect, it } from 'vitest';
import { eventEnvelope, SHORT_UUID_MATCHER, UUID_MATCHER } from '../../expect';
import { createRunner } from '../../runner';

it('Hono app captures errors (Hono SDK)', async ({ signal }) => {
it('Hono app captures parametrized errors (Hono SDK)', async ({ signal }) => {
const runner = createRunner(__dirname)
.expect(
eventEnvelope(
{
level: 'error',
transaction: 'GET /error',
transaction: 'GET /error/:param',
exception: {
values: [
{
Expand All @@ -24,12 +24,25 @@ it('Hono app captures errors (Hono SDK)', async ({ signal }) => {
request: {
headers: expect.any(Object),
method: 'GET',
url: expect.any(String),
url: expect.stringContaining('/error/param-123'),
},
breadcrumbs: [
{
timestamp: expect.any(Number),
category: 'console',
level: 'error',
message: 'Error: Test error from Hono app',
data: expect.objectContaining({
logger: 'console',
arguments: [{ message: 'Test error from Hono app', name: 'Error', stack: expect.any(String) }],
}),
},
],
},
{ includeSampleRand: true, sdk: 'hono' },
),
)

.expect(envelope => {
const [, envelopeItems] = envelope;
const [itemHeader, itemPayload] = envelopeItems[0];
Expand All @@ -39,7 +52,7 @@ it('Hono app captures errors (Hono SDK)', async ({ signal }) => {
expect(itemPayload).toMatchObject({
type: 'transaction',
platform: 'javascript',
transaction: 'GET /error',
transaction: 'GET /error/:param',
contexts: {
trace: {
span_id: expect.any(String),
Expand All @@ -51,15 +64,26 @@ it('Hono app captures errors (Hono SDK)', async ({ signal }) => {
},
request: expect.objectContaining({
method: 'GET',
url: expect.stringContaining('/error'),
url: expect.stringContaining('/error/param-123'),
}),
breadcrumbs: [
{
timestamp: expect.any(Number),
category: 'console',
level: 'error',
message: 'Error: Test error from Hono app',
data: expect.objectContaining({
logger: 'console',
arguments: [{ message: 'Test error from Hono app', name: 'Error', stack: expect.any(String) }],
}),
},
],
});
})

.unordered()
.start(signal);

await runner.makeRequest('get', '/error', { expectError: true });
await runner.makeRequest('get', '/error/param-123', { expectError: true });
await runner.completed();
});

Expand Down
20 changes: 18 additions & 2 deletions packages/hono/src/cloudflare/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,35 @@
import { withSentry } from '@sentry/cloudflare';
import { applySdkMetadata, type BaseTransportOptions, debug, type Options } from '@sentry/core';
import { applySdkMetadata, type BaseTransportOptions, debug, type Integration, type Options } from '@sentry/core';
import type { Context, Hono, MiddlewareHandler } from 'hono';
import { requestHandler, responseHandler } from '../shared/middlewareHandlers';

export interface HonoOptions extends Options<BaseTransportOptions> {
context?: Context;
}

const filterHonoIntegration = (integration: Integration): boolean => integration.name !== 'Hono';

export const sentry = (app: Hono, options: HonoOptions | undefined = {}): MiddlewareHandler => {
const isDebug = options.debug;

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

applySdkMetadata(options, 'hono');
withSentry(() => options, app);

const { integrations: userIntegrations } = options;
withSentry(
() => ({
...options,
// Always filter out the Hono integration from user-provided integrations (or when nothing is specified).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Isn't that a general problem when using it twice? I wonder why there is no isInstrumented to prevent that. Sounds more like a bug to me

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is rather that the patched error handler for Hono that is set up in the Cloudflare SDK runs earlier and already sends the error to Sentry - and this one is not parametrized.

// The Hono integration is already set up by withSentry, so adding it again would cause double-capturing (and non-parametrized URLs).
integrations: Array.isArray(userIntegrations)
? defaults => [...defaults.filter(filterHonoIntegration), ...userIntegrations.filter(filterHonoIntegration)]
: typeof userIntegrations === 'function'
? defaults => userIntegrations(defaults).filter(filterHonoIntegration)
: defaults => defaults.filter(filterHonoIntegration),
}),
app,
);

return async (context, next) => {
requestHandler(context);
Expand Down
4 changes: 3 additions & 1 deletion packages/hono/src/shared/middlewareHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export function responseHandler(context: Context): void {
getIsolationScope().setTransactionName(`${context.req.method} ${routePath(context)}`);

if (context.error) {
getClient()?.captureException(context.error);
getClient()?.captureException(context.error, {
mechanism: { handled: false, type: 'auto.faas.hono.error_handler' },
});
}
}
77 changes: 77 additions & 0 deletions packages/hono/test/cloudflare/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,81 @@ describe('Hono Cloudflare Middleware', () => {
expect(middleware.constructor.name).toBe('AsyncFunction');
});
});

describe('filters Hono integration from user-provided integrations', () => {
const honoIntegration = { name: 'Hono' } as SentryCore.Integration;
const otherIntegration = { name: 'Other' } as SentryCore.Integration;

const getIntegrationsResult = () => {
const optionsCallback = withSentryMock.mock.calls[0]?.[0];
return optionsCallback().integrations;
};

it.each([
['filters Hono integration out', [honoIntegration, otherIntegration], [otherIntegration]],
['keeps non-Hono integrations', [otherIntegration], [otherIntegration]],
['returns empty array when only Hono integration provided', [honoIntegration], []],
])('%s (array)', (_name, input, expected) => {
const app = new Hono();
sentry(app, { integrations: input });

const integrationsFn = getIntegrationsResult() as (defaults: SentryCore.Integration[]) => SentryCore.Integration[];
expect(integrationsFn([])).toEqual(expected);
});

it('filters Hono from defaults when user provides an array', () => {
const app = new Hono();
sentry(app, { integrations: [otherIntegration] });

const integrationsFn = getIntegrationsResult() as (defaults: SentryCore.Integration[]) => SentryCore.Integration[];
// Simulates getIntegrationsToSetup: defaults (from Cloudflare) include Hono; result must exclude it
const defaultsWithHono = [honoIntegration, otherIntegration];
expect(integrationsFn(defaultsWithHono)).toEqual([otherIntegration, otherIntegration]);
});

it('filters Hono integration out of a function result', () => {
const app = new Hono();
sentry(app, { integrations: () => [honoIntegration, otherIntegration] });

const integrationsFn = getIntegrationsResult() as unknown as (
defaults: SentryCore.Integration[],
) => SentryCore.Integration[];
expect(integrationsFn([])).toEqual([otherIntegration]);
});

it('passes defaults through to the user-provided integrations function', () => {
const app = new Hono();
const userFn = vi.fn((_defaults: SentryCore.Integration[]) => [otherIntegration]);
const defaults = [{ name: 'Default' } as SentryCore.Integration];

sentry(app, { integrations: userFn });

const integrationsFn = getIntegrationsResult() as unknown as (
defaults: SentryCore.Integration[],
) => SentryCore.Integration[];
integrationsFn(defaults);

expect(userFn).toHaveBeenCalledWith(defaults);
});

it('filters Hono integration returned by the user-provided integrations function', () => {
const app = new Hono();
sentry(app, { integrations: (_defaults: SentryCore.Integration[]) => [honoIntegration] });

const integrationsFn = getIntegrationsResult() as unknown as (
defaults: SentryCore.Integration[],
) => SentryCore.Integration[];
expect(integrationsFn([])).toEqual([]);
});

it('filters Hono integration from defaults when integrations is undefined', () => {
const app = new Hono();
sentry(app, {});

const integrationsFn = getIntegrationsResult() as unknown as (
defaults: SentryCore.Integration[],
) => SentryCore.Integration[];
expect(integrationsFn([honoIntegration, otherIntegration])).toEqual([otherIntegration]);
});
});
});
Loading