Skip to content

fix(client): better error reporting on login/registration#2076

Merged
ellie merged 1 commit into
atuinsh:mainfrom
xvello:xvello/version-check
Jun 3, 2024
Merged

fix(client): better error reporting on login/registration#2076
ellie merged 1 commit into
atuinsh:mainfrom
xvello:xvello/version-check

Conversation

@xvello

@xvello xvello commented Jun 2, 2024

Copy link
Copy Markdown
Contributor

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

xvello commented Jun 2, 2024

Copy link
Copy Markdown
Contributor Author

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

@ellie ellie left a comment

Copy link
Copy Markdown
Member

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