Skip to content

Owner invite errors aren't being displayed in frontend #2084

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

Closed
carols10cents opened this issue Jan 1, 2020 · 1 comment · Fixed by #2380
Closed

Owner invite errors aren't being displayed in frontend #2084

carols10cents opened this issue Jan 1, 2020 · 1 comment · Fixed by #2380
Labels

Comments

@carols10cents
Copy link
Member

Extracted from #2081

To reproduce

  • In the web UI, try to invite a user that doesn't exist in crates.io's database, like ghost.
  • The UI will display "An invite has been sent to ghost".

If you inspect the network request that happens when the invite is submitted, the backend returns:

{"errors":[{"detail":"could not find user with login `ghost`"}]}

The expected behavior is that the error is displayed in the frontend.

To fix

This code looks like it should be displaying errors, but it's not and I'm not entirely sure why. One guess I have is that the status code returned is 200 because that's what (old versions of?) Cargo expects even if there's an error.

Because the response is nonstandard, we might have to do something nonstandard to handle it, perhaps like the login router does?

We do have a test for this, which is passing incorrectly, because the mirage configuration is returning a different response than the backend is. The mirage configuration needs to be updated to return status 200 but with the errors JSON.

@carols10cents carols10cents added C-bug 🐞 Category: unintended, undesired behavior A-ui E-help-wanted E-has-mentor labels Jan 1, 2020
@Zexbe
Copy link
Contributor

Zexbe commented Jan 21, 2020

I want to fix this issue. I can either have the front end work around the bug, or change the response code on the back end.

The oldest version of Cargo that can handle an error response code is 1.34
I am basing it on looking for code like this on the different branches
https://github.com/rust-lang/cargo/blob/bb3f2c59274dbf181f2d1a58c54486df4292ec2a/crates/crates-io/lib.rs#L345 (thank you sfackler for helping me find this)

I think the risk of changing it is low.

  1. It would only happen when the call fails, and add owner does not seem like it would be used that often automatically in places it would be expected to fail
  2. Since the response will be standard, more things will just work, like this issue

I feel that the API not using http error codes is bad, because it increases the difficulty in using it with different tools, and libraries.

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

Successfully merging a pull request may close this issue.

3 participants