Allow beforeBreadcrumb to return Promise

[rdsedmundo]

Problem Statement

I want to obtain the response payload for the a fetch breadcrumb which requires usage of Promises, however beforeBreadcrumb does not support it.

beforeBreadcrumb: async (breadcrumb, hint) => {
      if (!['fetch'].includes(breadcrumb.category)) {
        return breadcrumb;
      }

      let responseBody;
      const shouldForwardNetworkBodies = () => requestUrl.startsWith(window.location.origin);

        if (shouldForwardNetworkBodies()) {
          // save the response body
          try {
            responseBody = await hint.response.clone().text();
          } catch {}
        }
      }

      return {
        ...breadcrumb,
        data: {
          ...breadcrumb.data,
          responseBody,
        },
      };
    },

Solution Brainstorm

Allow a Promise to be used alongside the synchronous version.

[AbhiPrasad]

Hey @rdsedmundo, this would be a breaking change, so we can't do this by default.

Maybe we look at adding a lifecycle hook for beforeBreadcrumb that allows you to mutate async.

[rdsedmundo]

Thanks @AbhiPrasad. I'm assuming then from your message that there are no existing alternatives.

I tried looking at how Sentry Replays do it, but it seems it's a different mechanism.

https://github.com/getsentry/sentry-javascript/blob/da339b4ecca8e1377c3765fa27f6386ba38c437c/packages/replay/src/coreHandlers/util/fetchUtils.ts#L35-L41

https://github.com/getsentry/sentry-javascript/blob/da339b4ecca8e1377c3765fa27f6386ba38c437c/packages/replay/src/coreHandlers/util/addNetworkBreadcrumb.ts#L22-L28

[AbhiPrasad]

Ah there does seem to be a lifecycle hook you can use!

https://github.com/getsentry/sentry-javascript/blob/da339b4ecca8e1377c3765fa27f6386ba38c437c/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts#L53

So you would do:

Sentry.init({
 ...
});

const sentryClient = Sentry.getCurrentHub().getClient();

sentryClient?.on('beforeAddBreadcrumb',(breadcrumb, hint) => {
  // updateBreadcrumb is user defined and can be async
  void updateBreadcrumb(breadcrumb)
});

[rdsedmundo]

But this fn call beforeAddNetworkBreadcrumb even mentions that it has to be sync.

https://github.com/getsentry/sentry-javascript/blob/da339b4ecca8e1377c3765fa27f6386ba38c437c/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts#L75-L95

I think what it's doing is adding a new breadcrumb of some sort later via the replay.addUpdate(() => call I linked in the other comment, not editing the original one.

[AbhiPrasad]

There can be race conditions where the breadcrumb is not updated in time, but I think for the lifecycle hook case here you should be fine. I'll let @mydea confirm though.

[mydea]

Yes, so if you want to mutate the breadcrumb sent to sentry it has to be sync, as of now. Because we just continue after the hook is fired, there is no way to wait on async functions, which is why for replay we only add stuff to the original breadcrumb that we can get sync, and do the async stuff separately (=and do not add e.g. body to the basic breadcrumbs).

I think if you really need this, the way to go is:

1. Disable fetch breadcrumbs in the Breadcrumbs integration - so opt out of the default fetch breadcrumb generation:

Sentry.init({
  integrations: [new Sentry.Breadcrumbs({ fetch: false })]
})

2. Basically reimplement this on your own, like this:

import * as Sentry from '@sentry/browser';
import { addInstrumentationHandler } from '@sentry/utils';

addInstrumentationHandler('fetch', fetchHandler);

You can look at https://github.com/getsentry/sentry-javascript/blob/develop/packages/browser/src/integrations/breadcrumbs.ts#L253 for how the default fetch handler is implemented!

[getsantry[bot]]

This issue 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 remove the label Waiting for: Community, I will leave it alone ... forever!

---

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