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

Discussion: NVD API client should not retry/continue on JsonProcessingExceptions? #167

Closed
chadlwilson opened this issue Jun 29, 2024 · 0 comments · Fixed by #168
Closed

Comments

@chadlwilson
Copy link
Contributor

Currently whenever there is a JsonProcessingException (specific cause JsonMappingException the library current gets stuck in a loop retrying to retrieve and parse data that is never going to work.

This leads to confusion like mentioned in jeremylong/DependencyCheck#6747 and jeremylong/DependencyCheck#6746 since the errors are hidden and it looks like NVD API problems. This is exacerbated on Gradle, because enabling debug logging to see the problem is non-trivial and in bigger projects will cause Gradle to freak out as it produces too many logs (e.g when using dependencyCheckAggregate). Additionally, I believe it creates a storm of unnecessary extra load on the NVD API?

This was changed in the below, but no real reason shared as to why.

825977e#diff-13b186ee3d6fa6c024eff401d1250ba1e32d00a02464e586aba44bd84015ff66L311-R315

Can we re-evaluate why this was changed, and consider reverting? Generally I'd have thought a JsonProcessingException is a fatal, unretryable error, and the library should fast fail with a clear error.

Since there are multiple reasons for JsonProcessingException perhaps the concern was retrying on temporary stream read issues. If that is the case, an alternative is to fail fast on JsonMappingException but to retry on others, e.g StreamReadException. Not convinced that is the concern though, as the code seems to parse from a string in memory, fully extracted from the body text, not stream it into the parser:

json = response.getBodyText();
// resolve issue #20
if (json == null && response.getBody().isBytes()) {
json = new String(response.getBodyBytes(), StandardCharsets.UTF_8);
}
CveApiJson20 current;
try {
current = objectMapper.readValue(json, CveApiJson20.class);
this.indexesToRetrieve.remove(call.getStartIndex());
} catch (JsonProcessingException e) {
LOG.debug("Error processing NVD data", e);
// really re-fetch the same data?
return next();
}

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 a pull request may close this issue.

1 participant