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(cli): Prevent panics in "wasmer login" after API failures #4148

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

theduke
Copy link
Contributor

@theduke theduke commented Aug 14, 2023

Previously login_and_save_token() was called from an async context.
Since the function uses a blocking reqwest client for the internal
GraphQL query, this could lead to panics when the API query failed, due
to the blocking reqwest tokio runtime trying to shut down inside another
tokio runtime, which is not allowed.

This is fixed by running login_and_save_token() inside a spawned thread.

Closes #4147

Copy link
Contributor

@Michael-F-Bryan Michael-F-Bryan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Previously login_and_save_token() was called from an async context.
Since the function uses a blocking reqwest client for the internal
GraphQL query, this could lead to panics when the API query failed, due
to the blocking reqwest tokio runtime trying to shut down inside another
tokio runtime, which is not allowed.

This is fixed by running login_and_save_token() inside a spawned thread.

Closes #4147
@theduke theduke force-pushed the issue-4147-fix-cli-login-panic branch from 4e3b8a6 to a8b66be Compare August 14, 2023 12:45
@theduke theduke enabled auto-merge August 14, 2023 12:55
@theduke theduke disabled auto-merge August 14, 2023 12:55
@theduke theduke enabled auto-merge (rebase) August 14, 2023 12:55
@theduke theduke merged commit 29c4730 into master Aug 15, 2023
@theduke theduke deleted the issue-4147-fix-cli-login-panic branch August 15, 2023 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime shutdown panic in "wasmer login"
2 participants