fix(source): reject HTTP error responses in UrlSource#25
fix(source): reject HTTP error responses in UrlSource#25
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: This PR modifies business logic and error handling in a core path (UrlSource initialization). Such changes require human review to confirm the behavior change is intended.
Architecture diagram
sequenceDiagram
participant Caller as Test Runner / Client
participant Src as UrlSource::new()
participant HTTP as reqwest (Blocking)
participant Srv as Remote Server / WireMock
Caller->>Src: Initialize with URL
Src->>HTTP: get(url)
HTTP->>Srv: HTTP GET Request
Srv-->>HTTP: HTTP Response (Status + Body)
HTTP-->>Src: Response Object
Note over Src: NEW: Validate HTTP Status
alt Status is SUCCESS (2xx)
Src->>HTTP: text()
HTTP-->>Src: Response Body (String)
Src-->>Caller: Ok(UrlSource)
else CHANGED: Status is FAILURE (4xx, 5xx, etc.)
Src->>Src: anyhow::bail!("HTTP {status} from {url}")
Src-->>Caller: Err(Error)
end
Note over Caller,Srv: Integration tests now verify 404 and 500 rejection
UrlSource::new()was callingreqwest::blocking::get()without checking the status code. A 404 or 500 response got silently treated as valid content, so you'd end up hashing an HTML error page instead of getting an error.Added a status check before reading the body. Updated the existing
test_url_source_http_500_succeeds(which was testing the broken behavior) and added a 404 test.Closes #16