Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions packages/cloudflare/src/withSentry.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type { env } from 'cloudflare:workers';
import { setAsyncLocalStorageAsyncContextStrategy } from './async';
import type { CloudflareOptions } from './client';
import { isInstrumented, markAsInstrumented } from './instrument';
import { getHonoIntegration } from './integrations/hono';
import { instrumentExportedHandlerEmail } from './instrumentations/worker/instrumentEmail';
import { instrumentExportedHandlerFetch } from './instrumentations/worker/instrumentFetch';
import { instrumentExportedHandlerQueue } from './instrumentations/worker/instrumentQueue';
import { instrumentExportedHandlerScheduled } from './instrumentations/worker/instrumentScheduled';
import { instrumentExportedHandlerTail } from './instrumentations/worker/instrumentTail';
import { getHonoIntegration } from './integrations/hono';

/**
* Wrapper for Cloudflare handlers.
Expand All @@ -20,7 +21,7 @@ import { instrumentExportedHandlerTail } from './instrumentations/worker/instrum
* @returns The wrapped handler.
*/
export function withSentry<
Env = unknown,
Env = typeof env,
Copy link
Member

Choose a reason for hiding this comment

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

q: Isn't this theoretically breaking?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really see this as a breaking change, as Env is only there so it gets inferred by the third generic entry, which I think just grew over time. So there are couple of usages and non of them look breaking to me:

// sets "Env" directly, so it isn't affected at all
withSentry<{ myEnv: string }>()

// "Env" is manually set, which means it is the same type now
withSentry((env: Env) => ({}), exportedHandler)

// Not setting anything, this wasn't working before, but works with this PR
withSentry(env => ({}), exportedHandler)

It could also be that people used not existing env variables, which wouldn't work as the exported handler is already typed, as documented, with the env usually (which is the same behavior with and without this PR):

const exportedHandler = { fetch() {} } satisfies ExportedHandler<Env>

withSentry((env: { notExistingEnv: string }) => ({}), exportedHandler)
                                                      ^^^^^^^^^^^^^^^

QueueHandlerMessage = unknown,
CfHostMetadata = unknown,
T extends ExportedHandler<Env, QueueHandlerMessage, CfHostMetadata> = ExportedHandler<
Expand Down
27 changes: 15 additions & 12 deletions packages/cloudflare/test/withSentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@ import { withSentry } from '../src/withSentry';
import { markAsInstrumented } from '../src/instrument';
import * as HonoIntegration from '../src/integrations/hono';

type HonoLikeApp<Env = unknown, QueueHandlerMessage = unknown, CfHostMetadata = unknown> = ExportedHandler<
declare global {
namespace Cloudflare {
interface Env {
SENTRY_DSN: string;
}
}
}

type HonoLikeApp<Env = Cloudflare.Env, QueueHandlerMessage = unknown, CfHostMetadata = unknown> = ExportedHandler<
Env,
QueueHandlerMessage,
CfHostMetadata
Expand All @@ -16,11 +24,6 @@ type HonoLikeApp<Env = unknown, QueueHandlerMessage = unknown, CfHostMetadata =
errorHandler?: (err: Error) => Response;
};

const MOCK_ENV = {
SENTRY_DSN: 'https://public@dsn.ingest.sentry.io/1337',
SENTRY_RELEASE: '1.1.1',
};

describe('withSentry', () => {
beforeEach(() => {
vi.clearAllMocks();
Expand All @@ -33,15 +36,15 @@ describe('withSentry', () => {
const handleHonoException = vi.fn();
vi.spyOn(HonoIntegration, 'getHonoIntegration').mockReturnValue({ handleHonoException } as any);

const honoApp = {
const honoApp: HonoLikeApp = {
fetch(_request, _env, _context) {
return new Response('test');
},
onError() {},
errorHandler(err: Error) {
return new Response(`Error: ${err.message}`, { status: 500 });
},
} satisfies HonoLikeApp<typeof MOCK_ENV>;
};

withSentry(env => ({ dsn: env.SENTRY_DSN }), honoApp);

Expand All @@ -59,13 +62,13 @@ describe('withSentry', () => {

const error = new Error('test hono error');

const honoApp = {
const honoApp: HonoLikeApp = {
fetch(_request, _env, _context) {
return new Response('test');
},
onError() {},
errorHandler: originalErrorHandlerSpy,
} satisfies HonoLikeApp<typeof MOCK_ENV>;
};

withSentry(env => ({ dsn: env.SENTRY_DSN }), honoApp);

Expand All @@ -86,13 +89,13 @@ describe('withSentry', () => {

markAsInstrumented(originalErrorHandler);

const honoApp = {
const honoApp: HonoLikeApp = {
fetch(_request, _env, _context) {
return new Response('test');
},
onError() {},
errorHandler: originalErrorHandler,
} satisfies HonoLikeApp<typeof MOCK_ENV>;
};

withSentry(env => ({ dsn: env.SENTRY_DSN }), honoApp);

Expand Down
Loading