Fix/update retry logic in OSV

[another-rex]

Fixes #856

Fixes a couple of things with our OSV API retry logic:

• Now also retry non 200 responses, in addition to retrying on network errors. (is there a need to only retry for 500 >= responses? Current logic retrying all non 200)
• Fix batchquery code so that retries are not sending empty data.
• Increase maxRetryAttempts from 3 to 4
• Implement exponential backoff (actually polynomial/quadratic backoff)
• Correctly close the body after reading error message.
• Implement retries for determineversion API call

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 46.87500% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 62.19%. Comparing base (de95630) to head (df89acb).

Files	Patch %	Lines
pkg/osv/osv.go	46.87%	17 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #860      +/-   ##
==========================================
+ Coverage   62.16%   62.19%   +0.03%     
==========================================
  Files         144      144              
  Lines       11808    11817       +9     
==========================================
+ Hits         7340     7350      +10     
- Misses       3997     3998       +1     
+ Partials      471      469       -2     

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

[G-Rath]

(is there a need to only retry for 500 >= responses? Current logic retrying all non 200)

You should not be re-trying 4xx errors as they explicitly mean "client error" and I'm pretty sure they all mean "do not retry without adjustment" but that might not technically be true (though this says "most")

I'd say if there's ever a case the API expects to return a 4xx that should be fine to retry, that's a mistake

(dashes off to double check his 4xx error codes to make sure he's actually correct)

[another-rex]

yeah that's true, but I don't think it's possible to call the osv.go functions with values that could cause a 400 though (other than maybe 429 because you sent too many queries), so I'm not sure if it's worth putting in extra logic there.

[G-Rath]

"stick an issue in the backlog" 🤷

[cuixq]

Sorry for the late review!

[oliverchang]

LGTM with some minor comments.

[oliverchang]

should we add some random jitter to this?

[oliverchang]

nit: might also make a little more logical sense to have the sleep after the actual action. particularly if we add jitter.

[another-rex]

added some random jitter. I think it makes more sense at the start than the end, as this avoids having to add a if statement to check if this is the last loop, and avoids having to add i+1 as well.