Skip to content

[cloudflare] expose async.setAsyncLocalStorageAsyncContextStrategy() #15342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aroman opened this issue Feb 7, 2025 · 13 comments
Closed

[cloudflare] expose async.setAsyncLocalStorageAsyncContextStrategy() #15342

aroman opened this issue Feb 7, 2025 · 13 comments
Assignees

Comments

@aroman
Copy link

aroman commented Feb 7, 2025

Problem Statement

It's great you now have official support for cloudflare! But it does not work with Durable Objects, only simple Cloudflare workers.

The way the plugin was written assumes that all cloudflare workers export a Handler. This is an incorrect assumption; Durable Objects do not. As a result, we cannot call withSentry(), which means there is no way for us to call setAsyncLocalStorageAsyncContextStrategy() to set up the ALS store to get sentry to work.

Solution Brainstorm

I suggest either exposing setAsyncLocalStorageAsyncContextStrategy() for us to call it ourselves, or introducing a helper function similar to withSentry() that is capable of instrumenting a DurableObject, which does have a fetch() method, but not the way your SDK is written to assume it does.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Feb 7, 2025
@aroman
Copy link
Author

aroman commented Feb 7, 2025

cc @AbhiPrasad :)

@aroman
Copy link
Author

aroman commented Feb 8, 2025

I wrote a shim to workaround this limitation...

import { DurableObject } from '@cloudflare/workers-types';
import * as Sentry from '@sentry/cloudflare';
import { CloudflareOptions } from '@sentry/cloudflare';

type DOFetch = DurableObject['fetch'];
type Fetch<Env> = Parameters<ExportedHandlerFetchHandler<Env>>[0];

/**
 * Creates an instrumented fetch handler for Durable Objects that is compatible with Sentry error monitoring.
 *
 * As of February 2025, @sentry/cloudflare only supports the fetch handler signature used in Cloudflare Workers runtime.
 * However, Durable Objects use a different fetch signature that does not include the environment and context parameters
 * required by the Sentry SDK at runtime. This utility creates a compatibility layer between the two signatures.
 * @example
 * ```ts
 * class MyDurableObject implements DurableObject {
 *   constructor(private state: DurableObjectState, private env: Environment) {}
 *
 *   fetch = createInstrumentedFetch(
 *     async (request, env, ctx) => {
 *       // Your fetch handler logic here
 *       return new Response("OK");
 *     },
 *     (env) => ({
 *       dsn: env.SENTRY_DSN,
 *       environment: env.ENVIRONMENT
 *     }),
 *     this.env,
 *     this.ctx
 *   );
 * }
 * ```
 */
export function createFetchWithSentry<Env extends Environment>(
  fetch: ExportedHandlerFetchHandler<Env>,
  optionsCallback: (env: Env) => CloudflareOptions,
  env: Env,
  ctx: ExecutionContext
): DOFetch {
  const stubContext: ExecutionContext = {
    waitUntil: (promise) => ctx.waitUntil(promise),
    passThroughOnException() {},
    props: {},
  };

  // Note: although @cloudflare/sentry does expose wrapRequestHandler, it does
  // not correctly set up the AsyncLocalStorage context or expose any means to
  // do so, so we need to use the slightly more complex withSentry method.
  // See: https://github.com/getsentry/sentry-javascript/issues/15342
  const instrumentedHandler = Sentry.withSentry(optionsCallback, { fetch });

  const instrumentedFetch = (req: Fetch<Env>) =>
    instrumentedHandler.fetch(req, env, stubContext);

  return instrumentedFetch;
}

@andreiborza
Copy link
Member

Hey @aroman, thanks for filing this and providing a shim.

Abhi is currently ooo, but we'll pick this up once he's back.

@jahands
Copy link
Contributor

jahands commented Feb 11, 2025

It would be great if we could initialize Sentry with an arbitrary callback so that users are still able to use Sentry for handlers that aren't automatically instrumented yet (e.g. Workflows).

Something like:

const result = await Sentry.withSentry(
	{
		dsn: env.SENTRY_DSN,
	},
	async () => {
		// stuff that could error
	}
)

This would unblock a lot of use-cases that are stuck using toucan-js

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Feb 11, 2025
@andreiborza
Copy link
Member

Thanks, we'll consider the options once Abhi is back.

@timkelty
Copy link

timkelty commented Feb 18, 2025

Even without using Durable Objects, I am still getting this error when building my worker:

1: import { setAsyncContextStrategy, getDefaultCurrentScope, getDefaultIsolationScope } from '@sentry/core';
2: import { AsyncLocalStorage } from 'node:async_hooks';
            ^
3:
4: /**
error during build:
RollupError: "AsyncLocalStorage" is not exported by "__vite-browser-external", imported by "node_modules/@sentry/cloudflare/build/esm/async.js".

Note: I have compatibility_flags = [ "nodejs_compat" ] in my wrangler config as mentioned in the docs. I've also tried nodejs_als.

@tellisnz
Copy link

With the shim - is there any way when using wrangler dev to get get the tracing to behave the same as it does when deployed?

Please see https://github.com/tellisnz/sentry-workers-do for an example.

When running locally:

Image

When deployed:

Image

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Feb 22, 2025
@AbhiPrasad
Copy link
Member

Hey folks - back from OOO and taking a look at this issue alongside some other cloudflare issues.

I'll write something up to how we want to tackle this. We've learned some things since the initial release. Thanks for your patience

@Lms24 Lms24 self-assigned this Mar 3, 2025
@Lms24
Copy link
Member

Lms24 commented Mar 3, 2025

Hi, I just opened a PR (#15557) which exposes setAsyncLocalStorageAsyncContextStrategy() from @sentry/cloudflare. Turns out we need this ourselves in higher level SDKs 😅 I'm not familiar with durable objects or other CF concepts so not sure if this is all you need. Let me know if we should keep this issue open; otherwise I'll close it in a couple of weeks.

@Lms24 Lms24 removed their assignment Mar 3, 2025
@getsantry getsantry bot moved this to Waiting for: Community in GitHub Issues with 👀 3 Mar 3, 2025
@getsantry
Copy link

getsantry bot commented Mar 25, 2025

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Mar 25, 2025
@timkelty
Copy link

Not stale! We're still waiting on working solution. @AbhiPrasad any update?

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Mar 25, 2025
@lforst
Copy link
Member

lforst commented Mar 25, 2025

@timkelty I think we forgot to close this. @Lms24 added setAsyncLocalStorageAsyncContextStrategy in #15557 released in 9.4.0. Is there still anything not working for you?

@timkelty
Copy link

Ah, thanks @lforst. Missed that. Working great for me now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

8 participants