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

Fix telemetry unsupported async #1131

Merged
merged 4 commits into from
Aug 18, 2022
Merged

Conversation

DavidGOrtega
Copy link
Contributor

@DavidGOrtega DavidGOrtega commented Aug 16, 2022

yargs fail do not support await if used the app throws a strange uncontrolled ecmascript exception. Due to this unhandledexception and unhandledrejection events were triggered raising analytics three times that would not even execute doe the the ecmascript exception.

yargs seems also the throw the error after the fail function, something that it does not do it with the default handler.

This PR follow the amazing work that @0x2b3bfa0 did at #1124 to simplify the telemetry however the approach due to the mentioned error is a bit different

@DavidGOrtega DavidGOrtega temporarily deployed to internal August 16, 2022 14:19 Inactive
@0x2b3bfa0
Copy link
Member

Not tested (away from keyboard) but looks good. Have you considered then() and catch() instead of that bald-eagle-nested await block?

@DavidGOrtega
Copy link
Contributor Author

Have you considered then() and catch() instead of that bald-eagle-nested await block?

Which one?
in any case then should not even considered by any meaning

@DavidGOrtega DavidGOrtega temporarily deployed to internal August 16, 2022 14:35 Inactive
@DavidGOrtega DavidGOrtega temporarily deployed to internal August 16, 2022 14:39 Inactive
bin/cml.js Show resolved Hide resolved
@DavidGOrtega DavidGOrtega temporarily deployed to internal August 16, 2022 14:50 Inactive
@casperdcl casperdcl added bug Something isn't working technical-debt Refactoring, linting & tidying telemetry labels Aug 16, 2022
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 🎉

@0x2b3bfa0 0x2b3bfa0 merged commit d6c9a01 into master Aug 18, 2022
@0x2b3bfa0 0x2b3bfa0 deleted the fix/telemetry-unsupported-async branch August 18, 2022 11:04
0x2b3bfa0 added a commit that referenced this pull request Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working technical-debt Refactoring, linting & tidying telemetry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cml on failure is sending statistics three times cml on failure is printing a huge stacktrace
3 participants