Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload: switch success response from 200 OK to 201 Created. #3484

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Upload: switch success response from 200 OK to 201 Created.
This is required by the spec since DAP-04, but was missed previously. I
think it is likely that clients are effectively just looking for a
"success" (i.e. 2xx) status code.
branlwyd committed Nov 14, 2024
commit d979edc0cdfb6848da585105963795bccdb64b68
2 changes: 1 addition & 1 deletion aggregator/src/aggregator/http_handlers.rs
Original file line number Diff line number Diff line change
@@ -511,7 +511,7 @@ async fn upload<C: Clock>(
.insert(KnownHeaderName::AccessControlAllowOrigin, origin);
}

Ok(Status::Ok)
Ok(Status::Created)
}

/// Handler for CORS preflight requests to "/tasks/.../reports".
8 changes: 4 additions & 4 deletions aggregator/src/aggregator/http_handlers/tests/report.rs
Original file line number Diff line number Diff line change
@@ -92,7 +92,7 @@ async fn upload_handler() {
.run_async(&handler)
.await;

assert_eq!(test_conn.status(), Some(Status::Ok));
assert_eq!(test_conn.status(), Some(Status::Created));
assert!(test_conn.take_response_body().is_none());
}

@@ -110,7 +110,7 @@ async fn upload_handler() {
.with_request_body(duplicate_id_report.get_encoded().unwrap())
.run_async(&handler)
.await;
assert_eq!(test_conn.status(), Some(Status::Ok));
assert_eq!(test_conn.status(), Some(Status::Created));
assert!(test_conn.take_response_body().is_none());

// Verify that reports older than the report expiry age are rejected with the reportRejected
@@ -461,7 +461,7 @@ async fn upload_handler_error_fanout() {
.send()
.await
.unwrap();
assert_eq!(response.status().as_u16(), 200);
assert_eq!(response.status().as_u16(), 201);

// Use up the connection pool's connections to cause a transaction-level error in the next
// uploads.
@@ -750,6 +750,6 @@ async fn upload_client_early_disconnect() {
.to_string()
})
.collect::<HashSet<String>>(),
HashSet::from(["400".into(), "200".into()])
HashSet::from(["400".into(), "201".into()])
);
}