Skip to content

chore: capture response body for empty errors#276

Merged
srijan-deepsource merged 2 commits intomasterfrom
srijan-deepsource-patch-2
Mar 6, 2026
Merged

chore: capture response body for empty errors#276
srijan-deepsource merged 2 commits intomasterfrom
srijan-deepsource-patch-2

Conversation

@srijan-deepsource
Copy link
Contributor

This would help debug cases like CSP-836.

This would help debug cases like CSP-836.

Signed-off-by: Srijan Saurav <68371686+srijan-deepsource@users.noreply.github.com>
@deepsource-io
Copy link

deepsource-io bot commented Mar 6, 2026

DeepSource Code Review

We reviewed changes in 383bb7a...bfabe47 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

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.
@srijan-deepsource srijan-deepsource merged commit 92bfa5a into master Mar 6, 2026
5 checks passed
@srijan-deepsource srijan-deepsource deleted the srijan-deepsource-patch-2 branch March 6, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants