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

Add request information to Angular ssr events #12757

Closed
MarcoGlauser opened this issue Jul 4, 2024 · 6 comments
Closed

Add request information to Angular ssr events #12757

MarcoGlauser opened this issue Jul 4, 2024 · 6 comments
Assignees
Labels
Feature: SSR Package: angular Issues related to the Sentry Angular SDK Stale

Comments

@MarcoGlauser
Copy link

MarcoGlauser commented Jul 4, 2024

Problem Statement

We run Angular in ssr mode and use Sentry to track errors. The errors themselves come through, but they don't have any metadata associated, since the angular sdk expects to run in a browser.
image

It would be great, if Sentry would automagically collect metadata from the ssr environment.

Solution Brainstorm

We managed to get some meta information added but had to jump through quite a few hoops:

  1. Inject node integration token into angular:
    tokens.ts
import { type Integration } from '@sentry/types';

export const SSR_SENTRY_INTEGRATIONS = new InjectionToken<Integration[]>(
    'SSR_SENTRY_INTEGRATIONS'
);

server.ts

import * as Sentry from '@sentry/node';
...
            await commonEngine.render({
                ....
                providers: [
                    { provide: APP_BASE_HREF, useValue: baseUrl },
                    { provide: RESPONSE, useValue: res },
                    { provide: REQUEST, useValue: req },
                    {
                        provide: SSR_SENTRY_INTEGRATIONS,
                        useValue: [
                            sentry.requestDataIntegration(),
                            sentry.nodeContextIntegration(),
                        ],
                    },
                ],
            });
  1. Extend the sentry ErrorHandler
    constructor(
        @Optional() @Inject(SSR_SENTRY_INTEGRATIONS) private ssrIntegrations: Integration[],
        @Optional() @Inject(REQUEST) private request: Request
    ){ 
        super();
        if(this.ssrIntegrations){
          for (const integration of this.ssrIntegrations) {
            Sentry.addIntegration(integration);
          }
        }
    }

   handleError(error: any) {
      if (this.request) {
          Sentry
              .getIsolationScope()
              .setSDKProcessingMetadata({
                  request: this.request,
              });
      }
      super().handleError(error)
   }

The main challenge was that if we loaded the node integrations directly(or even lazy), we would get compilation errors, since the frontend code would complain that there's no node api. By injecting the integrations from server.ts, we managed to avoid those errors, since sentry node will never appear in a browser bundle.

I hope that makes some sense but I have no idea, how viable it is to integrate into the sdk.

@Lms24
Copy link
Member

Lms24 commented Jul 5, 2024

Hi @MarcoGlauser thanks for writing in!

At the moment we do not officially support SSR Angular apps. The @sentry/angular SDK is purely meant to be used on the client/browser-side. That being said, I think you're on the right track with using @sentry/node on the server side. I'm unfortunately not too familiar with SSR in Angular but what you should generally aim for is to call Sentry.init from @sentry/node on your server side (only there) and Sentry.init from @sentry/angular on the client side (and only there).

From my limited understanding about Angular SSR, I'd say server.ts sounds like a good place to init and configure everything Sentry related for the server.

Just to confirm: Did you get things to work in the end as you wanted?

I'd love to get a more complete picture about how you're currently setting up Sentry in an SSR Angular application. Maybe we can extract a guide out of this to make it available to others. I think having this documentation is the first step. Either we stop there and users have at least a guide what to do or we base an SSR-compatible SDK on the findings from it (this is usually how we go about developing meta/SSR framework SDKs).

Would you be interested in writing a small guide what to do? It could be just in this thread, in our docs or if you have your own blog or similar, that's also totally fine. In that case, I'd only ask if we may eventually link to it from our docs or copy some of the contents (with attribution of course) when we create an in-house guide.

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Jul 5, 2024
@Lms24 Lms24 added Package: angular Issues related to the Sentry Angular SDK Feature: SSR labels Jul 5, 2024
@MarcoGlauser
Copy link
Author

Hi @Lms24,
Yes, we got the basics like proper url detection and ok stacktraces with sourcemapping working with our approach.

Using only @sentry/node in ssr mode didn't work because Angular has it's own error handler and @sentry/angular does some magic to extract usable error messages and stack traces. We tried to combine the two but never managed to get something usable with limited time and knowledge of the sdks.
Because of that we initialize a @sentry/node instance on server.ts for express and server.ts errors only.
The application errors(even during ssr) are still handled by @sentry/angular. To add metadata to the error, we inject the @sentry/node integrations from the ssr side as outlined above.

The SDKs don't seem to mind that both are running at the same time but we also have the imports strictly separated.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 8, 2024
@Lms24 Lms24 self-assigned this Jul 8, 2024
@Lms24
Copy link
Member

Lms24 commented Jul 10, 2024

Ahh so in server.ts, you're basically calling Sentry.init from @sentry/node and providing the error handler from @sentry/angular?

If you don't mind, could you share your server.ts file? I'd like to look into this a bit more and it looks like you're already a few steps ahead :)

Sorry, you actually already shared this. I'm just wondering where theSentry.init call happens on the server side.

@MarcoGlauser
Copy link
Author

MarcoGlauser commented Jul 10, 2024

I can't easily share our server.ts, since we also inject things like redis caches and have some api routes that make the whole thing bigger than I'd like :/

In the server.ts file, we're initializing @sentry/node as explained in the express integration guide:
https://docs.sentry.io/platforms/javascript/guides/express/
The only special thing is that we run dotenv before initializing.

Since angular has it's own way of handling errors, we're only passing requestDataIntegration() and nodeContextIntegration() from @sentry/node to the @sentry/angular SentryErrorHandler.
We do this through angulars dependency injection. That's where the SSR_SENTRY_INTEGRATIONS Injection token comes in that accepts a list of sentry sdk integrations.
We then subclass the @sentry/angular angular error handler to add the injected integrations to the @sentry/angular instance.
In our application configuration we provide the subclassed errorhandler instead of calling Sentry.createErrorHandler()

With this setup, @sentry/angular tries to handle the ssr error and annotate metadata with the provided integrations.

I hope this helps. Let me know if anything is unclear. Sadly, angular ssr has been a bit neglected by the core devs and only recently started getting some love again. So it's very understandable that third party devs haven't invested heavily into the eco system.

We have found that @sentry/angular runs into an exception when running the extractHttpModuleError function, since ErrorEvent is not defined in nodejs.
https://github.com/getsentry/sentry-javascript/blob/3ad3bdd2e35baf737b7492fbc7009bea75d2feb5/packages/angular/src/errorhandler.ts#L42C30-L42C40

Should I open a separate issue for that or do you not intend for @sentry/angular to run on node in general?

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

Lms24 commented Jul 12, 2024

Thanks for the details!

Sadly, angular ssr has been a bit neglected by the core devs and only recently started getting some love again

Yup, I saw the recent improvements (also regarding their docs) but it looks like it's still rather niche overall. Still we should invest some time into investigating what we can do here to at least unblock folks wanting to use Sentry in Angular SSR applications.

We have found that @sentry/angular runs into an exception when running the extractHttpModuleError function, since ErrorEvent is not defined in nodejs. Should I open a separate issue for that [...]?

I think the fix should be fairly straight forward (i.e. guard with a typeof check). I'll open a quick PR. No need for a separate issue :)

do you not intend for @sentry/angular to run on node in general?

To be honest, not sure yet! For the moment we don't expect it to run in server environments at all. Could be that we'd eventually have to create a separate Angular SSR package.

@getsantry
Copy link

getsantry bot commented Nov 23, 2024

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 🥀

@getsantry getsantry bot added the Stale label Nov 23, 2024
@getsantry getsantry bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: SSR Package: angular Issues related to the Sentry Angular SDK Stale
Projects
Archived in project
Development

No branches or pull requests

3 participants