Skip to content
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.

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

TypeError: Sentry.moduleMetadataIntegration is not a function when using loader script #13803

Closed
3 tasks done
gilisho opened this issue Sep 25, 2024 · 22 comments · Fixed by #13817
Closed
3 tasks done
Labels
Package: browser Issues related to the Sentry Browser SDK

Comments

@gilisho
Copy link
Contributor

gilisho commented Sep 25, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

7.119.0

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

Similarly to #13297, I am using a loader script and moduleMetadataIntegration seems to be unavailable when using loader script.
I need this integration as advised for micro-frontend environment, and to use that solution I am migrating from Sentry SDK 5.30.0 to 7.119.0, which is already challenging.

Any reason to not have a dedicated bundle for this integration like other integration have?
This is a necessity for micro-frontend environment so I really hope there's a way to work around it .

Would appreciate your help in the matter. 🙏

Steps to Reproduce

Use loader script for 7.x version.
Have this script above:

<script>
  // Configure sentryOnLoad before adding the Loader Script
  // https://docs.sentry.io/platforms/javascript/install/loader/#custom-configuration
  window.sentryOnLoad = function () {
    console.log('sentryOnLoad')
    console.log(window.Sentry)
    window.Sentry.init({
      // moduleMetadataIntegration is a required integration for having the module_metadata available in runtime
      integrations: [window.Sentry.moduleMetadataIntegration()],
      // Solution for getting the route to the unhandled error.
      // This is the recommended solution for micro-frontend environment.
      beforeSend: function(event){
        if (event?.exception?.values?.[0].stacktrace.frames) {
          const frames = event.exception.values[0].stacktrace.frames;

            // Find the last frame with module metadata containing a DSN
          const framesModuleMetadata = frames
            .filter(
              (frame) => frame.module_metadata && frame.module_metadata.dsn,
            )
            .map((v) => v.module_metadata)

          const routeTo = framesModuleMetadata.slice(-1); // using top frame only - you may want to customize this according to your needs

          if (routeTo.length) {
            event.extra = {
              ...event.extra,
              ROUTE_TO: routeTo,
            };
          }
        }

        return event;
      }
    })
  }
</script>

Expected Result

Sentry is initialized successfully.

Actual Result

TypeError: Sentry.moduleMetadataIntegration is not a function

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 25, 2024
@github-actions github-actions bot added the Package: browser Issues related to the Sentry Browser SDK label Sep 25, 2024
@chargome
Copy link
Member

@gilisho I don't think there is a specific reason for that, we should include this in our CDN so you can lazy-load it in you application. I'll put this on our backlog for now, PRs are also welcome of course!

For your application I'm afraid the only option right now is to use the integration via npm installation of our SDK.

@gilisho
Copy link
Contributor Author

gilisho commented Sep 26, 2024

Hi @chargome !
I am very open to the idea of contributing a PR! Moreover, I will need it to be available in Major 7.
Should I open two PRs for both major 7 and major 8? can you guide me a bit on the process? Also, I assume the PR should be pretty similar to #13241?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 26, 2024
@chargome
Copy link
Member

@gilisho nice!

For general contributing guidelines check https://github.com/getsentry/sentry-javascript/blob/develop/CONTRIBUTING.md

So I think the FeedbackIntegration and ReplayIntegration are more complex cases but you could check for example any of these: https://docs.sentry.io/platforms/javascript/configuration/integrations/#2-load-from-cdn-with-lazyloadintegration for reference.

@Lms24 can you maybe chime in for handling v7 updates?

@gilisho
Copy link
Contributor Author

gilisho commented Sep 26, 2024

It seems that I need to make changes and create a PR against v7 branch as well as develop, right?
opened a draft PR against development #13817

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 26, 2024
@chargome
Copy link
Member

@gilisho yes once a pr on v7 is merged we can trigger a release

@gilisho
Copy link
Contributor Author

gilisho commented Sep 27, 2024

I made 2 PRs for both v8 and v7. LMK if something is missing.
This is the first-time I'm diving into Sentry's source code so I hope I'm not missing anything. 🙏

v8 PR - #13817
v7 PR - #13822

Since my platform is using v7 ATM, #13822 is in a higher priority for me. I'd like to know when this can get released to a new version. Waiting for this to get released. 🙏
Thank you!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 27, 2024
@chargome
Copy link
Member

@gilisho thank you! We'll take a look 👍

@gilisho
Copy link
Contributor Author

gilisho commented Oct 3, 2024

Thank you! I added the test you requested for #13817. LMK if something else is needed for both PRs.

Some concerns have been raised though:

  1. Checking with the bundle I produced locally for moduleMetadataIntegration, I can see that the module metadata is added to event.extra as I need, which is great. But, it only works when I use the unminified bundle. When I use the minified version, the module metadata is not added to the stacktrace frames. I've seen these kind of issues are possible, like Error: Custom driver not compliant when using minified offline integration #5527.

  2. Actually, I just realized it's not the only obstacle for the micro-frontend solution to work for my platform. The event is not being transferred to the right DSN because I'm having a similar issue now with TypeError: window.Sentry.makeMultiplexedTransport is not a function. Any suggestions? Could this also be included in the loader script alongside the other included transport utils like “makeFetchTransport”? I believe this would require a small change to export it here. Otherwise there's no way to use the frontend solution with loader script, and we use loader script since we want to lazy load.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Oct 3, 2024
@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Oct 8, 2024
@gilisho
Copy link
Contributor Author

gilisho commented Oct 29, 2024

Hi @mydea, could you take a look again at #13822?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Oct 29, 2024
@Lms24
Copy link
Member

Lms24 commented Oct 30, 2024

Hi, I see both PRs against v8 and v7 are still open. Let's try to get the v8 one in first and then take a look at v7. Sorry that this is taking a while but we're a bit busy and when it comes to adding build/release artifacts we need to be extra careful.

@gilisho
Copy link
Contributor Author

gilisho commented Nov 10, 2024

That's ok.
I'm still waiting for a review on v8 PR - #13817.
v7 PR is also ready for review - #13822.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 10, 2024
@lforst
Copy link
Member

lforst commented Nov 11, 2024

Hi, before we merge these PRs, would you mind explaining your use-case in a bit more detail?

Personally, something feels off about having microfrontends + the loader. We need to be a bit more gatekeep-y with what we add to the loader because of bundlesize.

Btw I am guessing you would also need the multiplexed transport?

@gilisho
Copy link
Contributor Author

gilisho commented Nov 11, 2024

Hi @lforst, thanks for the response!

would you mind explaining your use-case in a bit more detail?

I have one Sentry instance which is initialized using the Loader script in the host. The microfrontends are rendered inside this host and we use the micro-frontend solution for the host Sentry instance.
I understand you do not want to add this integration to the loader's bundle. This is not what I am aiming for.
All I want is to make sure moduleMetadataIntegration has a dedicated bundle that I can load lazily (or in v7 - not-lazily) in addition to the loader's script, just like it's possible for other integrations.

Btw I am guessing you would also need the multiplexed transport?

Yes, I have raised this concern earlier. @mydea already addressed that here and I am okay with the solution he proposed.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 11, 2024
@lforst
Copy link
Member

lforst commented Nov 11, 2024

@gilisho that makes sense. That is actually a valid use-case. Thanks for explaining! Let's get it merged then.

@gilisho
Copy link
Contributor Author

gilisho commented Nov 11, 2024

@mydea Hey, unfortunately I do not have permission to reopen this issue. This issue is actually dependent on both PRs #13817 and #13822. V8 PR (#13817) is merged now which is super awesome! 🥳
Can we merge #13822 as well for the same support on v7? That would be great! (we are using v7 ATM)

@mydea mydea reopened this Nov 11, 2024
mydea pushed a commit that referenced this issue Nov 11, 2024
…13822)

This PR fixes #13803. The corresponding PR for latest major version is
#13817.

I saw the bundles generation for integration is happening in
`@sentry/integrations`, so I added a corresponding file for
`modulemetadata` in this package that exports that integration from
`@sentry/core`, so that the bundle actually gets created for this
integration as needed.
@mydea
Copy link
Member

mydea commented Nov 11, 2024

I merged both PRs, thank you for opening them! We'll include these in the next releases.

@mydea mydea closed this as completed Nov 11, 2024
@gilisho
Copy link
Contributor Author

gilisho commented Nov 12, 2024

Thank you so much for all the help!
Do you know what is the ETA of the next release for v7?

@mydea
Copy link
Member

mydea commented Nov 12, 2024

We'll try to cut a release in the near future, can't give a concrete ETA but hopefully should not be too long!

Copy link
Contributor

A PR closing this issue has just been released 🚀

This issue was referenced by PR #13817, which was included in the 8.38.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: browser Issues related to the Sentry Browser SDK
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants