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(client): better error reporting on login/registration #2076

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

xvello
Copy link
Contributor

@xvello xvello commented Jun 2, 2024

Closes #2030 - unless you downgraded api.atuin.sh by mistake, this is misleading error reporting on non-200 responses:

  • Use the handle_resp_error helper on the registration and login code paths for consistency. Tune its strings to be a bit more generic, feel free to reword
  • Only check the server version if the request is a 200 (if we get a 503 from the CDN, the version check would run and wrongly report)
  • Remove the version fallback from ensure_version if the header is not preset. Since we bumped the major version, the fallback is not needed + the header has been present for 6 months

Tangentially related, should handle_resp_error directly call ensure_version now, to ensure a consistent behaviour?

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

@xvello
Copy link
Contributor Author

xvello commented Jun 2, 2024

Yes, my weekend has been very rainy, thank you very much ☔

Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!

Tangentially related, should handle_resp_error directly call ensure_version now, to ensure a consistent behaviour?

I imagine this would make more sense

@ellie ellie merged commit b05761b into atuinsh:main Jun 3, 2024
14 checks passed
@xvello xvello deleted the xvello/version-check branch June 3, 2024 10:42
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.

[Bug]: could not login due to version mismatch
2 participants