grafana / faro-web-sdk

The Grafana Faro Web SDK, part of the Grafana Faro project, is a highly configurable web SDK for real user monitoring (RUM) that instruments browser frontend applications to capture observability signals. Frontend telemetry can then be correlated with backend and infrastructure data for full-stack observability.
https://grafana.com/oss/faro/
Apache License 2.0
690 stars 62 forks source link

Importing TracingInstrumentation logs a handled error in Vue/Pinia as an unhandled promises rejection #566

Closed dyllandry closed 2 months ago

dyllandry commented 2 months ago

Description

Our Vue app uses Pinia to manage state. Some of our Pinia actions throw async errors but they are handled by the Vue components using then/catch.

After only importing the TracingInstrumentation from faro, the errors those actions throw are now logged to the console as unhandled promise rejections by zonejs, despite being handled in the vue component which called it. The then/catch flow still works, it's just getting logged by zone.js as an uncaught error.

This is causing an issue because we also use the console instrumentation and error instrumentation to pickup errors and send them to grafana. This means handled errors are now getting recorded as uncaught errors in our frontend observability dashboards.

Is there a clear reason for this? If not, I can try to create a small reproducible example.

Steps to reproduce

I haven't yet created a reproducible example yet, but may if that is needed to observe it firsthand.

Environment

Demo

In the below example, an uncaught error appears in the console for an expired JWT. We normally catch these errors and handle them without logging anything to console. We have a Pinia action called authenticate that a Vue component calls and catches any errors using then/catch syntax.

After importing TracingInstrumentation they get logged as uncaught errors. The first error is because zone.js calls console.error and the faro console instrumentation logs that. I'm not sure how exactly, but then it appears a second time.

Screenshot 2024-04-24 at 20 09 01

codecapitano commented 2 months ago

Thanks for exporting @dyllandry.

Are you still on Faro 1.3.6 seems you auto update (^1.*) so I assume you are running the most recent Faro 1version (atm 1.6.0)?

codecapitano commented 2 months ago

The web-tracing package use opentelemtry under the hood.

I had a look if similar issues are reported and found the following:

Do you maybe compile your code to ES2017+ ?

It seems there's a problem with Zone.js and ES2017+.

Please note that due to an issue with zone.js, the ZoneContextManager does not work with JS code targeting ES2017+. In order to use the ZoneContextManager, please transpile back to ES2015.

dyllandry commented 2 months ago

Thanks for exporting @dyllandry.

Are you still on Faro 1.3.6 seems you auto update (^1.*) so I assume you are running the most recent Faro 1version (atm 1.6.0)?

npm list | grep faro gives

├── @grafana/faro-web-sdk@1.3.9
├── @grafana/faro-web-tracing@1.2.4

I can confirm that targeting ES2015 fixes the issue. Thanks for finding that!

I see that you can pass an optional context manager to the tracing instrumentation. Do you know of an alternative context manager besides ZoneContextManager that can be used to bypass this issue?

codecapitano commented 2 months ago

Hi @dyllandry afaik there's only the ZoneContextManager and the default context manager available. https://opentelemetry.io/docs/languages/js/getting-started/browser/#creating-a-tracer-provider

codecapitano commented 2 months ago

@dyllandry I don't think we can do much more here. Can we close the issue or do you want to keep it open for a while to make further insights available to other readers?

dyllandry commented 2 months ago

@codecapitano If the ZoneContextManager is all that's available then we'll stick with setting the target to ES2015 to fix it. Thanks for looking into this.

I think the issue can be closed. From the other tickets I didn't get a sense that zone.js will be changed to support async/await. Changing the compilation target to not include promises looks like the way to go.