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

Implement ErrorSink method #421

Merged
merged 5 commits into from Jul 29, 2021
Merged

Implement ErrorSink method #421

merged 5 commits into from Jul 29, 2021

Conversation

lestrrat
Copy link
Collaborator

@lestrrat lestrrat commented Jul 29, 2021

closes #420

@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2021

Codecov Report

Merging #421 (8d21888) into main (9204f79) will increase coverage by 0.15%.
The diff coverage is 74.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #421      +/-   ##
==========================================
+ Coverage   69.75%   69.90%   +0.15%     
==========================================
  Files          80       80              
  Lines        9227     9250      +23     
==========================================
+ Hits         6436     6466      +30     
+ Misses       1939     1930       -9     
- Partials      852      854       +2     
Impacted Files Coverage Δ
jwk/refresh.go 77.20% <74.57%> (+4.07%) ⬆️
jwk/jwk.go 59.07% <0.00%> (+0.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9204f79...8d21888. Read the comment docs.

@rockspore
Copy link

rockspore commented Jul 29, 2021

This seems to be working well. Thanks!

A question that may not be closely related: if the URL is misconfigured at the first place, it seems the refresh loop will never happen asynchronously even after a Fetch() is invoked? The snapshot indicates it stays at:

last refreshed at 0001-01-01 00:00:00 +0000 UTC, next refresh at 0001-01-01 00:00:00 +0000 UTC`

Is this desired?

nvm. It was the interval being too big for refresh to occur during the test.

@lestrrat lestrrat marked this pull request as ready for review July 29, 2021 20:52
@lestrrat lestrrat changed the title Implement FetchErrorChannel method Implement ErrorSink method Jul 29, 2021
FetchErrorChannel -> ErrorSink
AutoRefreshFetchError -> AutoRefreshError
@lestrrat
Copy link
Collaborator Author

@rockspore Thanks for checking. I just did a final renaming of the method to ErrorSink, and the error wrapper to AutoRefreshError, and am going to merge. I'm going to leave this in master and not release a new version for a few days. Let me know if you find any problems.

@lestrrat lestrrat merged commit c76a3f5 into main Jul 29, 2021
@lestrrat lestrrat deleted the gh-420 branch July 29, 2021 21:01
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.

None yet

3 participants