chore: capture response body for empty errors#276
Merged
srijan-deepsource merged 2 commits intomasterfrom Mar 6, 2026
Merged
Conversation
This would help debug cases like CSP-836. Signed-off-by: Srijan Saurav <68371686+srijan-deepsource@users.noreply.github.com>
|
|
Overall Grade |
Security Reliability Complexity Hygiene Coverage |
Feedback
- Failure-focused testing creates blind spots
- The test suite exercises many error branches with isolated mocks, leaving happy-path logic and integrations under-covered; introducing balanced success-path and integration tests would close those blind spots.
- Inconsistent error envelopes across layers
- Error context is captured ad hoc at each failure site, producing uneven logs and hard-to-aggregate diagnostics; a single error wrapper/schema at the service boundary would standardize observability and reduce duplication.
- Mock-heavy validation couples tests to implementation
- Rigorous failure-mode mocks verify internals but make tests brittle and miss real interactions; raising some tests to contract or integration level would validate observable behavior rather than internal call patterns.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go | Mar 6, 2026 11:51a.m. | Review ↗ | |
| Secrets | Mar 6, 2026 11:51a.m. | Review ↗ | |
| Test coverage | Mar 6, 2026 11:51a.m. | Review ↗ |
Code Coverage Summary
| Language | Line Coverage (New Code) | Line Coverage (Overall) |
|---|---|---|
| Aggregate | 100% [✓ above threshold] |
19.4% [▲ up 0.2% from master] |
| Go | 100% [✓ above threshold] |
19.4% [▲ up 0.2% from master][✓ above threshold] |
➟ Additional coverage metrics may have been reported. See full coverage report ↗
The previous commit introduced a bug where `queryResponseBody` was used instead of `responseBody`, causing a build failure. Additionally, when the upstream error was empty, the function would fall through and return "Reporting failed: " which is uninformative to callers. Now when the error is empty, we return the detailed error (including raw response) to the caller instead of just capturing it for monitoring. Added tests for createArtifact failure scenarios.
sourya-deepsource
approved these changes
Mar 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This would help debug cases like CSP-836.