fix(cloudflare): restore type inference for env parameter in withSentry - #22648
fix(cloudflare): restore type inference for env parameter in withSentry#22648mahenoorsalat wants to merge 1 commit into
Conversation
Closes getsentry#18294 Separates withSentry function signatures into overloads for ExportedHandler and WorkerEntrypoint so TypeScript properly infers the Env generic from the handler parameter.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca467a9. Configure here.
| export function withSentry( | ||
| optionsCallback: (env: any) => CloudflareOptions | undefined, | ||
| handler: any, | ||
| ): any { |
There was a problem hiding this comment.
Missing regression test for fix
Medium Severity
This is a fix PR, but the diff has no unit, integration, or E2E test covering the restored env inference. Per the testing conventions in the review rules, a regression test should fail without this overload change and pass with it—ideally a type-level assertion for the ExportedHandler path.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit ca467a9. Configure here.
| export function withSentry( | ||
| optionsCallback: (env: any) => CloudflareOptions | undefined, | ||
| handler: any, | ||
| ): any { |
There was a problem hiding this comment.
Uncommented any weakens Env typing
Medium Severity
The new WorkerEntrypoint overload and implementation signature introduce any for env, handler, and the return type without comments explaining why safer typing is impossible. That also drops env from typeof cloudflareEnv to any for entrypoint handlers, while instrumentWorkerEntrypoint already infers Env from the class constructor.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit ca467a9. Configure here.
JPeer264
left a comment
There was a problem hiding this comment.
That might work, but we need tests in order to proof it for the future. There are couple of different ways of writing the Cloudflare Worker bindings also there are other integrations relying on it like TanStack Start or Hono.
If you like you can add the tests, otherwise we can take over.
Possibilities:
// types inferred directly from the options
Sentry.withSentry((env: Env) => ({}), myHandler)
// types given as generic (we can drop that if necessary)
Sentry.withSentry<Env>(env => ({}), myHandler)
// types inferred from ExportedHandler
Sentry.withSentry(env => ({}), myHandler as ExportedHandler<Env>)
// types inferred from WorkerEntrypoint
class MyEntrypoint extends WorkerEntrypoint<Env> {}
Sentry.withSentry(env => ({}), MyEntrypoint)
// types from the global Cloudflare.Env (which is recommended by Cloudflare themselves)
class MyEntrypoint extends WorkerEntrypoint {}
Sentry.withSentry(env => ({}), MyEntrypoint)
// TanStack start. The `tanstackstart-react-clooudflare` E2E test ignores the types there
// so it is better to remove the `@ts-expect-error`
// also this needs to support the same variations as written above
export default Sentry.withSentry(env => ({}), wrapFetchWithSentry(handler));
// same for hono

Summary
Closes #18294
Fixes type inference for the
envparameter inwithSentryoptions callback.Previously, combining union generic parameters
ExportedHandler | WorkerEntrypointConstructorcaused TypeScript to fail type inference forEnv, defaultingenvtounknown/typeof cloudflareEnv.By overloading
withSentryseparately forExportedHandlerandWorkerEntrypointConstructor, TypeScript correctly infersEnvfrom the passed handler.Changes
packages/cloudflare/src/withSentry.ts: Added function overloads forwithSentryforExportedHandlerandWorkerEntrypointConstructor.