Support different JS runtimes

AbhiPrasad commented 2 years ago

Problem Statement

The Sentry SDK should work with WinterCG-compatible runtimes other than NodeJS. We should build new SDKs for these platforms, or figure out how to adapt our Node SDK to fit them.

The spec: https://github.com/wintercg/proposal-common-minimum-api

There are also other JS runtimes we can take a look at:

Bun: https://github.com/getsentry/sentry-javascript/discussions/7979

Solution Brainstorm

If you are interested in this, please comment on the individual issues linked above!

timfish commented 2 years ago

It looks like there's talk of WinterCG TypeScript definitions but they don't appear to have been added. Having a lib.wintercg.d.ts in TypeScript would probably be a prerequisite to ensuring proper library compatibility.

I've not looked too deeply yet but running the code through deno bundle gives us a good idea of where there might be issues. @sentry/core seems mostly compatible with WinterCG runtimes @sentry/utils is where the issues lay

instrument.ts contains a load of DOM/Browser types, XMLHttpRequest, Element, HTMLElement, etc
- We probably need to pull browser specific instrumentation out of utils
Various places accessing global, window, process can trip up type checking and bundlers and getGlobalObject is already getting pretty nuts: https://github.com/getsentry/sentry-javascript/blob/249e64d02efc4ae60626aaaba892593616c9dec9/packages/utils/src/global.ts#L36-L46
- Should we pass the global object down from the Client to avoid error prone/bundler borking automatic detection?
Should node.ts not be in @sentry/node rather than in utils?
- We should try and remove the remaining usages of isNodeEnv and avoid all automatic platform detection entirely
Same goes for browser.ts

That's all I've got for now!

timfish commented 1 year ago

The first step in supporting other runtimes is getting core, hub, types and utils compiling without dom/node types:

[x] - Move @sentry/hub code to @sentry/core and keep @sentry/hub as a stub re-exporting types from core so it's not a breaking change
- 5823
[x] - getGlobalObject should be replaced with something more universal like this from core-js. globalThis is the global in WinterCG and browsers/node have supported it for a while too
- 5809
- 5831
- 5884
- 5885
- 5914
- 5949
[ ] - Remove DOM and node TypeScript lib types from packages that shouldn't be using them
- 5834
- 5816

At this point, core, hub and types are compiling without DOM/node lib types.

However, utils still requires DOM and node lib types to build successfully.

To rectify this, all runtime specific code should be moved to the runtime packages:

[ ] - Move all browser specific code from @sentry/utils to @sentry/browser/utils
- [ ] - Move supportsErrorEvent, supportsDOMError, supportsDOMException, supportsFetch, supportsNativeFetch, supportsReferrerPolicy and supportsHistory to @sentry/browser
- [ ] - Move some functions (like getLocationHref and getDomElement) from browser.ts to @sentry/browser
- [ ] - Move everything in instrument.ts to @sentry/browser
  - ❌ This is an issue due to addInstrumentationHandler usage in tracing
[ ] - Move all node specific code from @sentry/utils to @sentry/node
- [ ] - Move everything in node.ts to @sentry/node
  - ❌ Moving dynamicRequire is an issue since it's used in tracing

The above can done without technically making breaking changes since we make no guarantees about @sentry/utils, but it will impact downstream SDKs that make use of some of the moved functions.

The major downside to moving these utilities to the browser/node packages and exporting them at the root is that they would become public API methods (for downstream SDKs to use) and with that comes stability guarantees.

It would be less of a maintenance headache to either:

Move them to new packages (like @sentry/browser-utils) or package export (@sentry/browser/utils)
Wrap them in an internal object export that we make no stability promises about

`@sentry/tracing`

There are a few issues above where @sentry/tracing relies on code from @sentry/utils which will get moved to browser/node. This will result in tracing depending on both browser/node. This can further be fixed by splitting the tracing code #5815.

lforst commented 1 year ago

How about having these as named exports? @sentry/tracing, @sentry/tracing/browser and @sentry/tracing/node?

Shouldn't it be @sentry/browser/tracing instead of @sentry/tracing/browser? So we can depend on @sentry/browser from within @sentry/browser/tracing. Maning the tracing submodule in @sentry/browser is just an extension of @sentry/browser. (I worded that kinda confusing but I think it gets the point across).

Also, I'd just have all tracing functionality in @sentry/node by default, so there wouldn't be any need for @sentry/node/tracing.

timfish commented 1 year ago

Yep, I need to update the above ramblings.

There's a more recent issue for the tracing changes.

Also, I'd just have all tracing functionality in @sentry/node by default, so there wouldn't be any need for @sentry/node/tracing.

Would we want to add all the hub extensions by default too?

We wouldn't want to run _autoloadDatabaseIntegrations in the Electron SDK because it hits app startup time significantly!

lforst commented 1 year ago

Continued thread over in more recent issue here: https://github.com/getsentry/sentry-javascript/issues/5815#issuecomment-1258309921

AbhiPrasad commented 1 year ago

We had a discussion internally about the global object changes - @timfish could we look into if we can just use globalThis?

timfish commented 1 year ago

could we look into if we can just use globalThis?

Eventually, but we'll need to wait for Node.js v12 to be our MSV.

And the Sentry supported browsers suggests we need to support Android 4.4 and IE10/11 so these would need to be polyfilled.

That means we can probably remove the window and self lines from this:

https://github.com/getsentry/sentry-javascript/blob/8570f0d29567e5f7cebf9a18f82e019191f2ebee/packages/utils/src/global.ts#L56-L65

lobsterkatie commented 1 year ago

LOL

Looks like we haven't updated this page in quite a long time. (EDIT: Heh. Turns out July-me already discovered that: https://github.com/getsentry/sentry-docs/issues/5351)

I can say for sure we don't need to worry about Android 4.4. I did a quick google, and the first version which can handle, for example, object spread is Android 8. For IE11, we've already told people that they have to use one of our ES5 bundles, to which we could add a polyfill if necessary. But the Node 10 business is definitely a bigger blocker. I like your idea of at least removing the browser-specific parts.

AbhiPrasad commented 1 year ago

We merged in https://github.com/getsentry/sentry-javascript/pull/5831, could we rebase the rest of the PRs @timfish?

pbteja1998 commented 1 year ago

I’m using Remix with Cloudflare Workers. It would be great if there is support for this.

There is this package that everyone uses when using Cloudflare Workers, it might be a good starting point to start work on Cloudflare Workers runtime. https://github.com/robertcepa/toucan-js

huw commented 1 year ago

I realised I didn’t post cross-post here last time—I have a pretty hacky solution for Cloudflare Workers with Toucan.

KrisBraun commented 1 year ago

🤞 for Cloudflare Pages support, which should be nearly identical to Cloudflare Workers, just with different handlers.

lforst commented 1 year ago

@KrisBraun Gonna happen soon™ 👀

huw commented 1 year ago

I’ve updated my Cloudflare Workers Gist for Toucan v3.2.0 and Remix v2. It’s a lot more complicated & hacky now thanks to Remix v2, but still very possible. Please reply there if you have any questions :)

AbhiPrasad commented 1 month ago

With @sentry/cloudflare being available in 8.22.0 and us having SDKs for @sentry/vercel-edge, @sentry/bun, @sentry/deno + exposing a bunch of APIs via @sentry/core, I think we can close this issue - we support every major non-node runtime, and we have all the building blocks via generic JS APIs in the SDKs for you to support any custom runtime you wish.

The one's left on the list are:

but right now these are not high priority for us. We'll tackle them slowly over the next couple quarters (PRs are always welcome though!)

getsentry / sentry-javascript