Conversation
WalkthroughFixed HTTP response body handling in FOFA and Hunter query agents by adding deferred body closure and simplifying error handling to remove redundant raw payload emission on JSON decode failures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
sources/agent/hunter/hunter.go (1)
82-86: Consider using a more appropriate log level for body close errors.Using
Info()for an error condition is unconventional. Body close errors are typically low-priority but still indicate something went wrong. Consider usingWarning()orDebug()depending on how visible you want this to be.defer func(Body io.ReadCloser) { if bodyCloseErr := Body.Close(); bodyCloseErr != nil { - gologger.Info().Msgf("response body close error : %v", bodyCloseErr) + gologger.Warning().Msgf("response body close error: %v", bodyCloseErr) } }(resp.Body)sources/agent/fofa/fofa.go (1)
96-98: Simplified error handling on decode failures.The removal of raw response body attachment on JSON decode errors simplifies the code but reduces debugging information for malformed API responses. This is acceptable for the current fix, though capturing a preview of the response body in the error message could help future troubleshooting.
If you want to enhance debugging without complexity, consider:
if err := json.NewDecoder(resp.Body).Decode(fofaResponse); err != nil { - results <- sources.Result{Source: agent.Name(), Error: err} + results <- sources.Result{Source: agent.Name(), Error: fmt.Errorf("decode error: %w", err)} return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sources/agent/fofa/fofa.go(1 hunks)sources/agent/hunter/hunter.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
sources/agent/fofa/fofa.go (2)
sources/agent/fofa/response.go (1)
FofaResponse(4-12)sources/result.go (1)
Result(8-17)
sources/agent/hunter/hunter.go (2)
sources/agent/hunter/response.go (1)
Response(17-21)sources/result.go (1)
Result(8-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Builds (macOS-latest, 1.24.x)
- GitHub Check: Test Builds (ubuntu-latest, 1.24.x)
- GitHub Check: Test Builds (windows-latest, 1.24.x)
- GitHub Check: release-test
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
sources/agent/hunter/hunter.go (1)
88-92: LGTM!Using
json.NewDecoder(resp.Body).Decode()directly is the idiomatic approach for decoding JSON from HTTP responses. This avoids the intermediate buffer allocation and simplifies error handling. The removal of raw payload emission on decode errors is appropriate since malformed JSON data isn't useful to downstream consumers.sources/agent/fofa/fofa.go (1)
89-93: Critical fix: Ensures response body stays open during decode.This defer pattern correctly addresses the regression in issue #708. By deferring the body close until after the JSON decode completes (line 96), the decoder can now fully read the response body, fixing the "empty results" bug in v1.2.1.
The error logging on close failure is also good defensive practice.
closes #708
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.