[IGNORE ME] chore: make hub explicit

[yordis]

trafficstars

As usual, ignore me 💜 so take it with a grain of salt.

Context

Removing global Hub and singletons, or at least pushing them to the edge of the library, should make things more testable without leaning into monkey patching or weird setups.

Also, it will allow you multiple Hubs if you want it since you push from the Core up to the edge, such as singleton.

[yordis]

I love that I came across something that got my attention, and a comment is in line with my first reaction.

Because I broke the API and passed the Hub https://github.com/getsentry/sentry-javascript/blob/7a6c3553fd35d841e293979b066154d0cd015b6b/packages/core/src/baseclient.ts#L258

The current implementation doesn't follow the interface because I must open the interface to accept a Hub (ignore why for now).

My first reaction was, Why is the client expecting a Hub? I was curious. I wanted to learn what the client was doing with the Hub until I realized the client was an interface.

So, the subsequent reaction was: alright, if that is the case, why client stuff has to do with setting things up since it feels that is something internal, which indeed it was: https://github.com/getsentry/sentry-javascript/blob/7a6c3553fd35d841e293979b066154d0cd015b6b/packages/types/src/client.ts#L103 Notice the comment.

So from this, it feels odd the shape of the interface and probably either needs more types or some functions to abstract away doing the right thing with integrations. In other words, some setup function knows how to hook the integrations, the clients, and whatnot instead of each client having to understand it...

It feels architecturally confusing. Also, I haven't looked at other clients from another ecosystem, but avoiding singletons until you must lead to the same conclusions regardless of the programming language.

As usual, no clue what I am doing, so take it with a grain of salt.

Also, any thoughts about that?

[yordis]

@AbhiPrasad @lobsterkatie @smeubank, any thoughts? (whenever you have time to deal with my curiosity obviously)

[yordis]

Alright, based on the following finding, it feels that the Hub should be the one with the state of the integrations instead of the Client.

Notice that the Hub calls the Client, and the client calls some function that doesn't use the Client, just the configuration of the this._options.integrations and this._integrationsInitialized = true, which could be a state in the Hub, and you avoid this back and forward.

But then it leads to some questions.

Was the intention of the Hub to have multiple clients? Since https://github.com/getsentry/sentry-javascript/blob/7a6c3553fd35d841e293979b066154d0cd015b6b/packages/core/src/integration.ts#L67 only allow a singleton installation. It sounds like you can't.

Can you swap the client at runtime? Even if that is possible, the integrations will not be set up again, so it doesn't matter. So probably not for the use case of setting up integrations thou.

So, it feels that moving the state is valid here. And from the architectural perspective kind of makes sense since you don't interact with the client directly, no? you would send a message to the Hub, and it will figure things out.

Another reason, https://github.com/getsentry/sentry-javascript/blob/7a6c3553fd35d841e293979b066154d0cd015b6b/packages/core/src/integration.ts#L1-L3

Such a file uses nothing from the core package.

[yordis]

Ignore what I said, I think the core is the place, and where things are confusing is in https://github.com/getsentry/sentry-javascript/blob/7a6c3553fd35d841e293979b066154d0cd015b6b/packages/core/src/sdk.ts#L16 I believe... that is where the in-and-out to multiple components start becoming confusing.

[yordis]

I wonder how many times initAndBind function suppose to be called in the lifecycle of the app.

So far, only per environment client (browser, node, test)

[yordis]

Enough for tonight,

Conclusion: (😄 after all): Hub is the global thing, the reason everything is global is because of the re-exporting of hub package from the SDKs, probably you need to modes at the edge, so things would become

import { init, initSigleton } from '...'

const sentry = init({ .... })

export { sentry }

So everywhere that people use the sentry SDK, the need to use the right Sentry instance:

import { sentry } from './my-sentry'

sentry.captureException();

// or

captureException(sentry); 

Wanna see how big of a mess is to make things to work to push the state a bit higher, in case of falling back to window global stuff, then you can have something that exports a function that returns a global Sentry setup.

import { getGlobalSentry } from '@sentry/globathis-hub';

getGlobalSentry().captureException();

// or

captureException(getGlobalSentry()); 

[yordis]

Also, really helpful: https://develop.sentry.dev/sdk/unified-api/#concurrency

[yordis]

Also related to https://github.com/getsentry/sentry-javascript/issues/3268 since that is what this would give a opportunity to solve in some sense, but still, problematic with global integration installation and whatnot, so .... at least

[lforst]

I couldn't agree more.

This is ofc very very far into the future bc it's a hard breaking change but I think we should remove all global bindings/instances/whatever. It's a bit of an anti-pattern in modern JS.

Even though it would break the unified API, I think returning some sort of Sentry instance from Sentry.init() would be the best move to end the multi-hub problem once and for all.

I actually wanna hear the team's opinion on this.

[yordis]

The multi-hub kind of makes sense, but having the issue of global processors and monkey patches that wouldn't work with more than one instance is the big question.

Before even getting to the Hub section, such installations use the Hub, so should they be aware of multi-hubs?

Webworker and stuff like that could benefit from opening the API.

---

Side question, what was the motivation for creating the hub and types packages instead of moving that into core?

[github-actions[bot]]

This pull request 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 label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

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

[yordis]

The multi-hub kind of makes sense, but having the issue of global processors and monkey patches that wouldn't work with more than one instance is the big question.

Before even getting to the Hub section, such installations use the Hub, so should they be aware of multi-hubs?

Still need help with that one 🤔

[github-actions[bot]]

This pull request 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 label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

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