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

Add: Retrying for NVDApi #1070

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Add: Retrying for NVDApi #1070

wants to merge 7 commits into from

Conversation

n-thumann
Copy link
Member

@n-thumann n-thumann commented Dec 5, 2024

What

This PR adds the retrying of requests to NVDApi and the inheriting classes.

Why

NVD is super unstable lately, so it's nearly impossible to retrieve a large response, because one of the HTTP requests will very likely fail. This caused vt-cve-library to be broken right now and cve-feeder to occasionally fail.

The current behavior on main is that the caller needs to handle those errors. However, the caller doesn't know which request (the offset of the paginated response) exactly failed, so send a new request starting from failed requests offset.
One option would be to somehow pass this offset to the caller, however I opted for the approach to handle retries directly inside Pontos, just like rate-limiting also is for a more out-of-the-box developer experience.

This implementation is backwards-compatible and assumes one attempt, if not specified otherwise.
If attempts is greater than 1 and there is a server-side (HTTP 5xx) error in the response, the request will be retried after a delay of 2 seconds, exponentially backing off.

In my tests for vt-cve-library I was finally able to request data from NVD again, which failed after a few minutes before this change.

References

Checklist

  • Tests

Copy link

github-actions bot commented Dec 5, 2024

Conventional Commits Report

Type Number
Changed 2
Added 5

🚀 Conventional commits found.

Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.92%. Comparing base (10a5bf5) to head (fd81880).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1070      +/-   ##
==========================================
+ Coverage   89.91%   89.92%   +0.01%     
==========================================
  Files         108      108              
  Lines        7178     7187       +9     
  Branches      812      815       +3     
==========================================
+ Hits         6454     6463       +9     
  Misses        521      521              
  Partials      203      203              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@n-thumann n-thumann added the make release To trigger GitHub release action. label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
make release To trigger GitHub release action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant