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

Make error in authenticator a little clearer #282

Merged
merged 2 commits into from
Mar 5, 2025
Merged

Conversation

einarmo
Copy link
Collaborator

@einarmo einarmo commented Feb 28, 2025

There isn't a ton we can do to make it obvious what went wrong, since we can't really log the response from an OAuth request, but we can at least make it obvious that the error came from the authenticator. Currently you'll just get "Error in middleware: Failed to deserialize" which is less helpful.

@einarmo einarmo requested a review from a team as a code owner February 28, 2025 07:35
@einarmo einarmo requested review from ikezedev and a team February 28, 2025 07:35
@einarmo einarmo requested review from a team and rbtcollins and removed request for a team March 5, 2025 12:18
@@ -119,7 +119,7 @@ impl StatusCode {
/// Set the info type, this will clear the info bits if set to NotUsed.
#[must_use = "Status code is copied, not modified in place."]
pub fn set_info_type(mut self, value: StatusCodeInfoType) -> Self {
self.0 = self.0 & !(1 << 10) | ((value as u32) & 1) << 10;
self.0 = self.0 & !(1 << 10) | (((value as u32) & 1) << 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just for clarity? It clearly isn't affecting tests; so either the tests are insufficient, or the evaluation is unchanged.

(I don't have all the operator priorities for this in my head, sorry ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Evaluation is unchanged, this is for clarity, required by a new clippy lint. Since we're building in github actions we get new rust versions automatically, which occasionally causes this.

I'm reasonably certain changing the precedence here would fail one of the tests for this type.

@rbtcollins
Copy link
Contributor

Assuming the () change is just for clarity.... approved

@rbtcollins
Copy link
Contributor

🦄

@einarmo einarmo merged commit d47464d into master Mar 5, 2025
3 checks passed
@einarmo einarmo deleted the clarify-auth-error branch March 5, 2025 17:02
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.

3 participants