feat(browser): Add `setBrowserErrorFrameAsyncContextStrategy` #7807

AbhiPrasad · 2023-04-11T10:05:05Z

Based on @kamilogorek's excellent work in https://github.com/getsentry/sentry-js-tracing/blob/master/src/tracer-vanilla.ts, we add a async context strategy for the browser.

This means that we can get hub isolation in the browser as well - as long as people wrap stuff with runWithAsyncContext.

@timfish curious to get your thoughts on this.

timfish · 2023-04-11T10:17:45Z

Oh nice!

Should we remove the SENTRY_HUB_ID_ frames from stack traces?

Could you export a version of runWithAsyncContext from the browser SDK that ensures that setBrowserErrorFrameAsyncContextStrategy gets called so users don't need to worry about this step?

The other strategies are still missing this too but I think we should add some tests that tests it all works with async/await.

timfish · 2023-04-11T10:20:37Z

packages/browser/src/async.ts

+    return {
+      [fnName]: (cb: (hub: Hub) => T) => {
+        hubs.set(hubId, newHub);
+        return cb(newHub);


hubs will grow in size indefinitely. You need to remove the entry after the callback is called and also in the case of error.

kamilogorek · 2023-04-11T10:32:50Z

Damn, how many legacy ideas have I left behind!? So glad you found it! :D

AbhiPrasad · 2023-04-11T10:33:12Z

Hmm for some reason this stops working when I do something like so:

test.only('hub scope getCurrentHub', async () => {
  async function expensiveOperation() {
    return Array.from({ length: 1e6 }, (_, i) => i).reduce((acc, v) => acc + v);
  }

  setBrowserErrorFrameAsyncContextStrategy();

  const globalHub = getCurrentHub();
  globalHub.setExtra('hub', 'global');

  await runWithAsyncContext(async hub1 => {
    hub1.setExtra('hub', '1');
    return runWithAsyncContext(async hub2 => {
      hub2.setExtra('hub', '2');
      return await expensiveOperation().then(() => {
        expect(getCurrentHub()).toBe(hub2);
      });
    });
  });
});

Possibly the promise is causing the error to get lost?

timfish · 2023-04-11T10:39:15Z

Possibly the promise is causing the error to get lost?

Quite possibly. What does the stack look like at every context level?

AbhiPrasad · 2023-04-11T10:56:04Z

Doing something like so:

await runWithAsyncContext(async _ => {
  console.log('hub1', new Error());
  return runWithAsyncContext(async _ => {
    console.log('hub2', new Error());
    return await expensiveOperation().then(() => {
      console.log('hub2-again', new Error());
    });
  });
});

Gets me:

hub1 Error: 
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:56:27)
        at Generator.next (<anonymous>)
        at /Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:115:75
        at new Promise (<anonymous>)
        at Object.__awaiter (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:111:16)
        at cb (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:55:41)
        at Object.SENTRY_HUB_ID_0 (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/src/async.ts:50:16)
        at Object.runWithAsyncContext (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/src/async.ts:52:14)
        at Object.runWithAsyncContext (/Users/abhijeetprasad/workspace/sentry-javascript/packages/core/src/hub.ts:600:1)
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:55:11)
        at Generator.next (<anonymous>)
        at /Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:115:75
        at new Promise (<anonymous>)
        at Object.__awaiter (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:111:16)
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:45:51)
        at Promise.then.completed (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/utils.js:391:28)
        at new Promise (<anonymous>)
        at callAsyncCircusFn (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/utils.js:316:10)
        at _callCircusTest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:218:40)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at _runTest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:155:3)
        at _runTestsForDescribeBlock (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:66:9)
        at _runTestsForDescribeBlock (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:60:9)
        at run (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:25:3)
        at runAndTransformResultsToJestFormat (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:170:21)
        at jestAdapter (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:82:19)
        at runTestInternal (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-runner/build/runTest.js:389:16)
        at runTest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-runner/build/runTest.js:475:34)
        at TestRunner.runTests (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-runner/build/index.js:101:12)
        at TestScheduler.scheduleTests (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/@jest/core/build/TestScheduler.js:333:13)
        at runJest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/@jest/core/build/runJest.js:404:19)

      at Object.<anonymous> (test/unit/async.test.ts:56:12)

  console.log
    hub2 Error: 
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:58:29)
        at Generator.next (<anonymous>)
        at /Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:115:75
        at new Promise (<anonymous>)
        at Object.__awaiter (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:111:16)
        at cb (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:57:44)
        at Object.SENTRY_HUB_ID_1 (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/src/async.ts:50:16)
        at Object.runWithAsyncContext (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/src/async.ts:52:14)
        at Object.runWithAsyncContext (/Users/abhijeetprasad/workspace/sentry-javascript/packages/core/src/hub.ts:600:1)
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:57:14)
        at Generator.next (<anonymous>)
        at /Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:115:75
        at new Promise (<anonymous>)
        at Object.__awaiter (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:111:16)
        at cb (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:55:41)
        at Object.SENTRY_HUB_ID_0 (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/src/async.ts:50:16)
        at Object.runWithAsyncContext (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/src/async.ts:52:14)
        at Object.runWithAsyncContext (/Users/abhijeetprasad/workspace/sentry-javascript/packages/core/src/hub.ts:600:1)
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:55:11)
        at Generator.next (<anonymous>)
        at /Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:115:75
        at new Promise (<anonymous>)
        at Object.__awaiter (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/node_modules/tslib/tslib.js:111:16)
        at Object.<anonymous> (/Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:45:51)
        at Promise.then.completed (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/utils.js:391:28)
        at new Promise (<anonymous>)
        at callAsyncCircusFn (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/utils.js:316:10)
        at _callCircusTest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:218:40)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at _runTest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:155:3)
        at _runTestsForDescribeBlock (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:66:9)
        at _runTestsForDescribeBlock (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:60:9)
        at run (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/run.js:25:3)
        at runAndTransformResultsToJestFormat (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:170:21)
        at jestAdapter (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:82:19)
        at runTestInternal (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-runner/build/runTest.js:389:16)
        at runTest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-runner/build/runTest.js:475:34)
        at TestRunner.runTests (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/jest-runner/build/index.js:101:12)
        at TestScheduler.scheduleTests (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/@jest/core/build/TestScheduler.js:333:13)
        at runJest (/Users/abhijeetprasad/workspace/sentry-javascript/node_modules/@jest/core/build/runJest.js:404:19)

      at Object.<anonymous> (test/unit/async.test.ts:58:7)

  console.log
    hub2-again Error: 
        at /Users/abhijeetprasad/workspace/sentry-javascript/packages/browser/test/unit/async.test.ts:60:37
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        ```

AbhiPrasad · 2023-04-11T10:57:20Z

I guess the only way around this is to monkey patch patch promises themselves 😬

timfish · 2023-04-11T11:07:20Z

I think this is a consequence of then losing the previous stack. If you stick to awaiting this doesn't happen.

AbhiPrasad · 2023-04-11T12:15:42Z

I think this is a consequence of then losing the previous stack. If you stick to awaiting this doesn't happen.

Yup, it's because of https://v8.dev/blog/fast-async#improved-developer-experience

AbhiPrasad · 2023-04-14T19:02:29Z

Closing this because it not working with promise.then seems to be a dealbreaker 😢.

Maybe one day.

feat(browser): Add setBrowserErrorFrameAsyncContextStrategy

40d26d2

AbhiPrasad requested a review from timfish April 11, 2023 10:05

timfish reviewed Apr 11, 2023

View reviewed changes

AbhiPrasad closed this Apr 14, 2023

AbhiPrasad deleted the abhi-browser-error-frame-strategy branch April 14, 2023 19:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(browser): Add `setBrowserErrorFrameAsyncContextStrategy` #7807

feat(browser): Add `setBrowserErrorFrameAsyncContextStrategy` #7807

AbhiPrasad commented Apr 11, 2023 •

edited

Loading

timfish commented Apr 11, 2023

timfish Apr 11, 2023

kamilogorek commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

timfish commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

timfish commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

AbhiPrasad commented Apr 14, 2023

feat(browser): Add setBrowserErrorFrameAsyncContextStrategy #7807

feat(browser): Add setBrowserErrorFrameAsyncContextStrategy #7807

Conversation

AbhiPrasad commented Apr 11, 2023 • edited Loading

timfish commented Apr 11, 2023

timfish Apr 11, 2023

Choose a reason for hiding this comment

kamilogorek commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

timfish commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

timfish commented Apr 11, 2023

AbhiPrasad commented Apr 11, 2023

AbhiPrasad commented Apr 14, 2023

feat(browser): Add `setBrowserErrorFrameAsyncContextStrategy` #7807

feat(browser): Add `setBrowserErrorFrameAsyncContextStrategy` #7807

AbhiPrasad commented Apr 11, 2023 •

edited

Loading