feat(core): Add async context abstraction

[timfish]

Adds AsyncContextStrategy abstraction to core.

I still have some major concerns over whether this should be in @sentry/core at all as it feels messy.

To cater for Node.js, the runWithHub signature in this PR does not actually suffice and would need to be something like below to cater for the existing domain code.

sentry-javascript/packages/node/src/handlers.ts

Lines 184 to 188 in a0e9489

	const local = domain.create();
	local.add(req);
	local.add(res);
	local.run(() => {

function runWithHub<T>(callback: (hub: Hub) => T, domainAdd: EventEmitter[] = []): T;

EventEmitter doesn't exist outside of Node.js, so we'd need to create some matching types or do some any or unknown fudging.

If/when the async context proposal lands and runWithHub gets exported from @sentry/browser we almost certainly wont want to export this node specific signature.

If we export platform specific getCurrentHub() and runWithHub() from their respective packages, no changes are required in core apart from removing the existing platform specific code and the platform specific packages get one additional export (ie. runWithHub).

[timfish]

Also, on the naming front, would it be better if runWithHub was called something like runWithAsyncContext to better describe what it's actually doing?

[AbhiPrasad]

Can we make runWithHub just pass through it's arguments? We lose the type safety, but gain the flexibility.

function runWithHub<T, A extends unknown[]>(callback: (hub: Hub, ...args: A) => T,  ...args: A): T;

[AbhiPrasad]

runWithAsyncContext

I like this - it's more backwards compat as well.

@lforst will be a fan :)

[lforst]

runWithAsyncContext

I like this - it's more backwards compat as well.

@lforst will be a fan :)

indeed <3

[timfish]

Can we make runWithHub just pass through it's arguments?

Yeah possibly, and then just if (arg instanceof EventEmitter) before passing the args to add as a safety check.

[timfish]

This is safer since we know all the args will be of the same type:

function runWithAsyncContext<T, A>(callback: (hub: Hub, ...args: A[]) => T, ...args: A[]): T;

[lforst]

This is safer since we know all the args will be of the same type:

function runWithAsyncContext<T, A>(callback: (hub: Hub, ...args: A[]) => T, ...args: A[]): T;
Maybe we'll need a default type of unknown for A 🤔

How about the following? I think this should work (at least as the user-facing type - internally we may need some casts):

function runWithAsyncContext<C extends (hub: Hub, ...args: any[]) => any>(callback: C): ReturnType<C>;

[timfish]

How about the following? I think this should work (at least as the user-facing type - internally we may need some casts):

function runWithAsyncContext<C extends (hub: Hub, ...args: any[]) => any>(callback: C): ReturnType<C>;

We need to pass the callback args as parameters to the top level function so we can use them in our AsyncContextStrategy implementation like this.

I tried this PR in its current state and replaced some run and bind usages with runWithAsyncContext and the types look good. For the single usage where these parameters are used (ie. the node request handler) it needs to be called with types (runWithAsyncContext<void, EventEmitter>() since request and response are different types that both inherit EventEmitter. Everywhere else where there aren't args passed through the types are not needed.

In SDKs where these additional arguments are not required or a bit smelly, we could re-export runWithAsyncContext with these additional parameters removed. Since they're optional parameters, it wont matter if users import from core or the SDK.