JuliaWeb / HTTP.jl

HTTP for Julia
https://juliaweb.github.io/HTTP.jl/stable/
Other
626 stars 177 forks source link

mark recoverability of DNS errors correctly #1088

Closed tanmaykm closed 11 months ago

tanmaykm commented 11 months ago

This changes HTTP client to not retry on certain DNS errors. From the list of possible DNS errors only EAI_AGAIN may be recoverable. This also changes the error code comparison to use UV_EAI_AGAIN, because that is what a DNSError instance would contain.

fixes: #1085

codecov-commenter commented 11 months ago

Codecov Report

Merging #1088 (f4bd74b) into master (10182c0) will decrease coverage by 0.02%. The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #1088      +/-   ##
==========================================
- Coverage   82.33%   82.32%   -0.02%     
==========================================
  Files          32       32              
  Lines        3040     3043       +3     
==========================================
+ Hits         2503     2505       +2     
- Misses        537      538       +1     
Files Changed Coverage Δ
src/clientlayers/RetryRequest.jl 79.16% <75.00%> (-0.84%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

nickrobinson251 commented 11 months ago

can we add a test please?

tanmaykm commented 11 months ago

We had faced this in a likely stressed condition in our application. I was testing this with a modified julia binary to artificially throw the DNS error. I found it hard to replicate the error otherwise.

I could add a unit test for isrecoverable, maybe that would be useful?

tanmaykm commented 11 months ago

@nickrobinson251 Actually I think I can add some relevant tests. And I also missed out some more changes to isrecoverable. PR in a bit.

nickrobinson251 commented 11 months ago

I think I can add some relevant tests

Thanks!

a unit test for isrecoverable, maybe that would be useful?

yeah, i think unit tests would be helpful