feat(remix): Add Worker Runtime support to Remix SDK.

[onurtemizkan]

This PR adds partial support to Remix SDK for worker-based runtimes.
It does not cover performance tracing.

Summary:

• Added a new transport which is compatible with both Cloudflare and Shopify Oxygen's fetch implementations.
• Replaced node-related dependencies of Remix SDK with worker-compatible alternatives.
• Updated browser detection logic in Remix SDK
• Added 3 E2E test applications:
  • remix-cloudflare-workers -> works on Miniflare runtime
  • remix-cloudflare-pages -> works on Miniflare runtime
  • remix-hydrogen -> works on MiniOxygen runtime
• Created a new initializer called workerInit, which is exported from index.client.ts as worker environments are preferring browser entries.

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)	66.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped)	56.21 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)	31.19 KB (0%)
@sentry/browser - Webpack (gzipped)	21.4 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)	62.65 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)	29.47 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)	21.55 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)	197.27 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)	89.15 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)	64.12 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)	32.12 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)	66.43 KB (0%)
@sentry/react - Webpack (gzipped)	21.45 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)	83.16 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)	48.32 KB (0%)
@sentry-internal/feedback - Webpack (gzipped)	16.19 KB (0%)

[huw]

I might be crying wolf but I think this is going to run into #4071 for Cloudflare Workers. According to Cloudflare’s docs:

A single Workers instance may handle multiple requests including concurrent requests in a single-threaded event loop. That means that other requests may (or may not) be processed during awaiting any async tasks (such as fetch) if other requests come in while processing a request.

There’s also this extended explanation from a Cloudflare staff member:

We re-use isolates as a performance optimization wherever possible. This means that as we process one request with set of environment bindings, another request also start to be processed, potentially with a different set of environment bindings. In the typical Cloudflare Worker written in ESM format, this isn't an issue since the request and env come in together. However, if you were to access the environment from a global scope like Next.js does, there's a risk that the new set of environment bindings will override the previous set as that previous request is still processing.

So, as far as I understand, if 2 concurrent requests come into a Worker in the same region, there’s a non-zero chance that they’ll run inside the same isolate. If one of these Workers is awaiting, then they’ll share the same global scope. This would be fine, because you could just check for an initialised Hub, but Cloudflare doesn’t support AsyncLocalStorage without enabling nodejs_compat, which means scopes/etc. will bleed into each other (unless there have been more changes since #4071 that I’m unaware of). AFAIK (again), this is why Cloudflare don’t recommend using global scope unless you know what you’re doing, and why Toucan avoids it. @robertcepa would know more—and I’d love to know if/where I’m wrong because having async context with solve all sorts of other problems for me in Workers :)

[timfish]

there’s a non-zero chance that they’ll run inside the same isolate. If one of these Workers is awaiting, then they’ll share the same global scope. This would be fine, because you could just check for an initialised Hub, but Cloudflare doesn’t support AsyncLocalStorage without enabling nodejs_compat, which means scopes/etc. will bleed into each other

You beat me to it but this was essentially going to be my review.

For example, with the following worker code:

export default {
  async fetch(request, env, ctx) {
    if(globalThis.counter === undefined){
      globalThis.counter = 0;
    }

    return new Response(`Hello World! ${globalThis.counter++}`);
  },
};

Every time you hit this endpoint from a similar location/timeframe, you'll see the global is not unique per request:
https://worker-icy-wind-4214.meaty.workers.dev/

If we want context isolation we would need to:

• Instruct users to enable node compatibility mode
• Create an AsyncContextStrategy that uses AsyncLocalStorage
  • I guess the existing node one won't work due to Cloudflare requiring the node: prefix
• Wrap each request with runWithAsyncContext

[huw]

(If I can add a personal note, I’d prefer a solution without nodejs_compat, but IDK what channels you have with Cloudflare—maybe they’d be open to adding AsyncContext to the native runtime? Or we can take a stab at refactoring some of the code here to accept a custom getCurrentHub() function, like you do with integrations today?)

[timfish]

Looking back through the PR again, the worker Client isn't using any integrations so it's currently only setup for capturing errors via Sentry.captureException(). That means the only context that would leak are usages of captureBreadcrumb, configureScope or setting context or tags, etc.

That means this PR adds support for reporting errors that was not previously available.

Or we can take a stab at refactoring some of the code here to accept a custom getCurrentHub() function, like you do with integrations today?

Even toucan-js doesn't instrument most things that Sentry does because there's currently no way to get the context. For example, for everything that Sentry instruments globally (fetch, console, etc), there's no way to know which request this event refers to without AsyncLocalStorage. Even if you forget all the global instrumentation (which isn't used in this PR), toucan-js only works by passing the client to anywhere you want to call the Sentry methods.

Maybe we could automatically add support for AsyncLocalStorage when node_compat is enabled?

[onurtemizkan]

Thanks a lot for the reviews @huw and @timfish!

So this PR addresses the lack of basic server-side error reporting for Remix versions/usages where handleError is available. We had previous discussions about handleError and decided to give the users Sentry.captureRemixServerException to report errors that end up in handleError (so leaving the global error-tracking to Remix) with a static context using Sentry.captureException internally.

The errors caught in instrumentBuild (which is mainly performance-tracing oriented, and we can't make work at the moment due to a limitation of Date.now()) are not covered by this update. The errors caught in global instrumentation of instrumentBuild will also end up in handleError for Remix v2, or v1 where v2 future flags are turned on. For custom contexts and capturing, we can advise the users to use node compatibility mode at this point (which won't work for hydrogen apps though) and we can try detecting that mode to avoid potential leaks between nodes.

Does that make sense? Am I missing any context around Cloudflare environments?

[huw]

👍😎👍 I think adding support for the nodejs_compat flag + clearly documenting the requirement would do it. If you do add support for nodejs_compat in getCurrentHub then I think the path to having a @sentry/worker package would be pretty straightforward and solve a host of other problems.

(But if anyone from Cloudflare is reading this—can you pretty please add AsyncContext to the main runtime?)

[AbhiPrasad]

Can we vendor this in if possible?

[onurtemizkan]

This has a dependency tree of 6 micro packages. May look ugly in the codebase.

[AbhiPrasad]

Going to close this now that we have https://docs.sentry.io/platforms/javascript/guides/cloudflare/frameworks/remix/