Skip to content

fix(core): Call getCurrentHubShim when accessing __SENTRY__.hub #12160

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.

Sign up for GitHub

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
wants to merge 4 commits into from
Closed

fix(core): Call getCurrentHubShim when accessing __SENTRY__.hub #12160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd10c2a
fix(core): Re-introduce `isOlderThan` to `AsyncContextStack`
Lms24 May 22, 2024
44e516a
add test
Lms24 May 22, 2024
4bf0949
use `stack` to set AsyncStackStrategy; use `hub` via `getCurrentHub`
Lms24 May 22, 2024
f3ddaac
size limit, biome
Lms24 May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .size-limit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module.exports = [
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'browserTracingIntegration', 'replayIntegration'),
gzip: true,
limit: '70 KB',
limit: '71 KB',
},
{
name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags',
Expand Down
7 changes: 7 additions & 0 deletions dev-packages/browser-integration-tests/suites/old-sdk-interop/hub/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://[email protected]/1337',
});
9 changes: 9 additions & 0 deletions dev-packages/browser-integration-tests/suites/old-sdk-interop/hub/subject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
console.log(window.__SENTRY__);
/**
* Simulate an old pre v8 SDK obtaining the hub from the global sentry carrier
* and checking for the hub version.
*/
const res = window && window.__SENTRY__ && window.__SENTRY__.hub && window.__SENTRY__.hub.isOlderThan(7);

// Write back result into the document
document.getElementById('olderThan').innerText = res;
9 changes: 9 additions & 0 deletions dev-packages/browser-integration-tests/suites/old-sdk-interop/hub/template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
</head>
<body>
<p id="olderThan"></p>
</body>
</html>
13 changes: 13 additions & 0 deletions dev-packages/browser-integration-tests/suites/old-sdk-interop/hub/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../utils/fixtures';

sentryTest(
"doesn't crash if older SDKs access `hub.isOlderThan` on the global object",
async ({ getLocalTestUrl, page }) => {
const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);

await expect(page.locator('#olderThan')).toHaveText('false');
},
);
26 changes: 12 additions & 14 deletions packages/core/src/asyncContext/stackStrategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { Client, Scope as ScopeInterface } from '@sentry/types';
import { isThenable } from '@sentry/utils';
import { getGlobalSingleton, isThenable } from '@sentry/utils';
import { getDefaultCurrentScope, getDefaultIsolationScope } from '../defaultScopes';
import { Scope } from '../scope';

import { getMainCarrier, getSentryCarrier } from './../carrier';
import { getMainCarrier, getSentryCarrier } from '../carrier';
import { getCurrentHubShim } from '../getCurrentHubShim';
import type { AsyncContextStrategy } from './types';

interface Layer {
Expand Down Expand Up @@ -131,20 +132,17 @@ export class AsyncContextStack {
* This will be removed during the v8 cycle and is only here to make migration easier.
*/
function getAsyncContextStack(): AsyncContextStack {
const registry = getMainCarrier();

// For now we continue to keep this as `hub` on the ACS,
// as e.g. the Loader Script relies on this.
// Eventually we may change this if/when we update the loader to not require this field anymore
// Related, we also write to `hub` in {@link ./../sdk.ts registerClientOnGlobalHub}
const sentry = getSentryCarrier(registry) as { hub?: AsyncContextStack };

if (sentry.hub) {
return sentry.hub;
// eslint-disable-next-line deprecation/deprecation
const sentry = getSentryCarrier(getMainCarrier()) as { hub?: AsyncContextStack };
if (!sentry.hub) {
// For now, we still set the `hub` object to gracefully handle pre v8 SDKs and certain scripts
// (e.g. our loader script) accessing properties on `window.__SENTRY__.hub`.
// This will just call the `getCurrentHubShim` function, as if users would interact with `getCurrentHub`.
// eslint-disable-next-line deprecation/deprecation
Object.defineProperty(sentry, 'hub', { get: () => getCurrentHubShim(), enumerable: true });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second oof: Referencing getCurrentHubShim here actually increases bundle size by +1% because previously, getCurrentHub(Shim) was completely optional :(

}

sentry.hub = new AsyncContextStack(getDefaultCurrentScope(), getDefaultIsolationScope());
return sentry.hub;
return getGlobalSingleton('stack', () => new AsyncContextStack(getDefaultCurrentScope(), getDefaultIsolationScope()));
}

function withScope<T>(callback: (scope: ScopeInterface) => T): T {
Expand Down
14 changes: 12 additions & 2 deletions packages/core/src/getCurrentHubShim.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Client, EventHint, Hub, Integration, IntegrationClass, SeverityLevel } from '@sentry/types';
import type { Client, EventHint, Hub, Integration, IntegrationClass, Scope, SeverityLevel } from '@sentry/types';
import { addBreadcrumb } from './breadcrumbs';
import { getClient, getCurrentScope, getIsolationScope, withScope } from './currentScopes';
import {
Expand All @@ -22,7 +22,10 @@ import {
* usage
*/
// eslint-disable-next-line deprecation/deprecation
export function getCurrentHubShim(): Hub {
export function getCurrentHubShim(): Hub & {
getStackTop: () => { client: Client | undefined; scope: Scope };
isOlderThan: () => boolean;
} {
return {
Comment on lines +25 to +28
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed: We actually export this publicly from @sentry/core (not sure why we did this at all). Do we care that we widen the return type here?

bindClient(client: Client): void {
const scope = getCurrentScope();
Expand Down Expand Up @@ -64,6 +67,13 @@ export function getCurrentHubShim(): Hub {
// only send the update
_sendSessionUpdate();
},
getStackTop: () => ({
client: getClient(),
scope: getCurrentScope(),
}),
isOlderThan() {
return false;
},
};
}

Expand Down
1 change: 1 addition & 0 deletions packages/utils/src/worldwide.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export interface InternalGlobal {
_sentryDebugIds?: Record<string, string>;
__SENTRY__: {
hub: any;
stack: any;
logger: any;
extensions?: {
/** Extension methods for the hub, which are bound to the current Hub instance */
Expand Down
Loading