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

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

Closed
dyllandry opened this issue Apr 25, 2024 · 6 comments
Labels
bug Report a bug

Comments

@dyllandry
Copy link

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

  • SDK version: ^1.3.6
  • SDK instrumentations: TracingInstrumentation, getWebInstrumentations
  • Device type: MacBookPro Apple M2 Max
  • OS: Ventura 13.6.6 (22G630)
  • Browser: Chrome 123.0.6312.106 (Official Build) (arm64)

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

@dyllandry dyllandry added the bug Report a bug label Apr 25, 2024
@codecapitano
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

codecapitano commented Apr 26, 2024

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
Copy link
Collaborator

@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
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report a bug
Projects
None yet
Development

No branches or pull requests

2 participants