ramosbugs / openidconnect-rs

OpenID Connect Library for Rust
MIT License
404 stars 100 forks source link

Include the full discovery url on invalid HTTP response #142

Closed Timshel closed 9 months ago

Timshel commented 9 months ago

Hey,

Simple PR to add the full discovery url to the error since in lot of cases it will be due to an improper issuer_url.

codecov[bot] commented 9 months ago

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (d796cca) 51.84% compared to head (c706e10) 51.76%.

Files Patch % Lines
src/discovery.rs 0.00% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #142 +/- ## ========================================== - Coverage 51.84% 51.76% -0.08% ========================================== Files 16 16 Lines 2145 2148 +3 ========================================== Hits 1112 1112 - Misses 1033 1036 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ramosbugs commented 9 months ago

Thanks for the PR! I'll merge once CI passes (looks like cargo fmt is unhappy)

Timshel commented 9 months ago

O yes sorry :)

Timshel commented 9 months ago

I had tried running it and it failed so I invalidly concluded that you did not use it. On another project it's included in the rust-toolchain.toml.

But I'm a bit of a rust noob so not sure of the best practices :).

ramosbugs commented 9 months ago

Thanks!

I had tried running it and it failed so I invalidly concluded that you did not use it.

When I run it locally with Cargo/Rust 1.71.0 (rustfmt 1.5.2-stable), it doesn't seem to change any of the formatting. Is it possible you ran it with the nightly toolchain?

On another project it's included in the rust-toolchain.toml.

I think this file is mainly for pinning to specific versions: https://rust-lang.github.io/rustup/overrides.html#the-toolchain-file.

ramosbugs commented 7 months ago

This is now released in 3.5.0.