chore(vulnerable-code): Add more details to an issue message #8210
Conversation
| issues += Issue(source = providerName, message = it.collectMessages()) | ||
| issues += Issue( | ||
| source = providerName, | ||
| message = "Failed to request chunk ${index + 1} of ${chunks.size} (chunk size ${chunk.size}): " + |
There was a problem hiding this comment.
What's the reason for adding these details? e.g. for what purpose could the index and the size of the chunk be used?
There was a problem hiding this comment.
It helped me debug aboutcode-org/vulnerablecode#1411.
There was a problem hiding this comment.
What do you think about alternatively doing all of these:
- leave the issue message as is (or at least do not add the chunk size)
- Create error / warning log message, which contains all
purlvalues contained in the batch / chunk which failed.
There was a problem hiding this comment.
or at least do not add the chunk size
What's the problem with adding the chunk size?
2. which contains all
purlvalues contained in the batch / chunk which failed.
That would have been 91 PURLs in this case, blowing up the log message quite a bit.
In all, I still prefer the current solution.
There was a problem hiding this comment.
In general: I believe that for debugging it's ok to look into the log output and that not all information needed for debugging needs to go into issue message. It suffices if it points one to the relevant bits of the logs IMO.
What's the problem with adding the chunk size?
I believe that the chunk size is not sufficient information anyway, so it would not bring much value in debugging, as one needs to dig deeper anyway.
That would have been 91 PURLs in this case, blowing up the log message quite a bit.
Apart from that, my assumption was (maybe I was wrong here) that one of the purls must have cause the
issue. So, I assumed that figuring out the root cause would involve making individual requests for each purl in the chunk, to figure out the problematic one. This is why I proposed to log the PURL''s which to me translates to logging the request which failed.
There was a problem hiding this comment.
Apart from that, my assumption was (maybe I was wrong here) that one of the purls must have cause the
issue.
While that turned out to be correct, my initial assumption was that the timeout is related to a too big chunk size, which is why I was interested in it.
Also, OkHttpClientHelper already logs these kind of errors in general, so I wanted to add some additional information here to the issue.
Anyway, to move forward, I've changed to code to what you proposed.
Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
vc-issue-message
branch
from
cdc8935 to
91613ff
Compare
No description provided.