Skip to content

Update client to convey per-report errors#4346

Open
jcjones wants to merge 1 commit intomainfrom
jcj/4146-client-per-report-errors
Open

Update client to convey per-report errors#4346
jcjones wants to merge 1 commit intomainfrom
jcj/4146-client-per-report-errors

Conversation

@jcjones
Copy link
Contributor

@jcjones jcjones commented Feb 12, 2026

After PR #3831, when HTTP 200 OK is returned with partial failures in the body, the client was treating it as complete success. This updates the client to properly parse the UploadResponse and return per-report errors.

Changes:

  • Add Error::Upload variant to convey per-report errors
  • Update put_report and upload_with_ohttp to parse and return UploadResponse
  • Check for per-report errors in upload_with_time even with HTTP 200 status
  • Add tests for per-report error handling

Fixes #4146

After PR #3831, when HTTP 200 OK is returned with partial failures in the
body, the client was treating it as complete success. This updates the
client to properly parse the UploadResponse and return per-report errors.

Changes:
- Add Error::Upload variant to convey per-report errors
- Update put_report and upload_with_ohttp to parse and return UploadResponse
- Check for per-report errors in upload_with_time even with HTTP 200 status
- Add tests for per-report error handling

Fixes #4146
@jcjones jcjones requested a review from a team as a code owner February 12, 2026 20:22
if let Some(upload_response) = upload_response {
let failed_reports = upload_response.status();
if !failed_reports.is_empty() {
return Err(Error::Upload(failed_reports.to_vec()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of a shame to have to copy the &[ReportUploadStatus] to Vec<ReportUploadStatus> since the enclosing struct gets dropped. We could do UploadResponse::take_status(self) -> Vec<ReportUploadStatus> and explicitly yoink the Vec. But honestly the compiler is probably smart enough to avoid the copy here, and it's not worth investing work until/unless we measure a performance problem here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be short, too, since it's only the failed jobs.

... should. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update client to convey per-report errors

2 participants