Harden parseErrorCodeFromS3 to never throw on non-XML responses by saintstack · Pull Request #12986 · apple/foundationdb

saintstack · 2026-04-14T16:36:27Z

Return "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Remove unused backup_parse_s3_response_failure error code. Fix const-ref parameter, constexpr badRequestCode, and add unit tests.

I ran the ctests against s3

      Start  2: s3client_test
 2/56 Test  #2: s3client_test .....................................   Passed   15.39 sec
      Start  3: bulkload_test
 3/56 Test  #3: bulkload_test .....................................   Passed  153.42 sec


....

      Start 12: s3_backup_tests
12/56 Test #12: s3_backup_tests ...................................   Passed   92.57 sec
      Start 13: s3_backup_bulkdump_bulkload_tests
13/56 Test #13: s3_backup_bulkdump_bulkload_tests .................   Passed  244.74 sec

Return "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Remove unused backup_parse_s3_response_failure error code. Fix const-ref parameter, constexpr badRequestCode, and add unit tests.

Copilot

Pull request overview

Hardens S3 error-code extraction so non-XML HTTP error bodies (e.g., 502/503 HTML pages or empty responses) do not raise errors, and adds regression tests to ensure parsing remains best-effort.

Changes:

Update parseErrorCodeFromS3 to return "" on parse failures instead of throwing, and add diagnostic tracing for non-XML parse exceptions.
Remove the now-unused backup_parse_s3_response_failure error code.
Minor cleanup (constexpr for badRequestCode) and add unit tests covering valid/invalid XML, empty, HTML, and garbage responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
flow/include/flow/error_definitions.h	Removes unused backup/restore error code tied to the old throwing behavior.
fdbclient/S3BlobStore.cpp	Makes S3 error parsing non-throwing, improves const-correctness, and adds unit tests for common non-XML failure modes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gxglass

I think this is a relatively contained PR and a good candidate for immediate backporting

foundationdb-ci · 2026-04-14T17:00:42Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:23:48
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T17:11:38Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 84cdb05
Duration 0:34:58
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T17:20:02Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 84cdb05
Duration 0:43:24
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T17:24:11Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 84cdb05
Duration 0:47:33
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T17:32:10Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:55:29
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T17:36:39Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:59:58
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

saintstack · 2026-04-14T17:37:30Z

Failures are tests/slow/GcGenerations.toml, tests/restarting/from_7.3.0/ConfigureTestRestart-1.toml, tests/restarting/from_7.3.0/ConfigureTestRestart-2.toml, tests/fast/PrivateEndpoints.toml. main is a bit dirty. These seem unrelated....

foundationdb-ci · 2026-04-14T18:07:57Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:29:56
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T18:21:25Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:43:20
Result: ❌ FAILED
Error: Error while executing command: ninja -v -C build_output -j ${NPROC} all packages strip_targets. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2026-04-14T18:22:44Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 84cdb05
Duration 0:44:40
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T18:24:51Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 84cdb05
Duration 0:46:47
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T18:49:27Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 84cdb05
Duration 1:11:22
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T19:16:51Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 84cdb05
Duration 1:38:51
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

saintstack · 2026-04-14T19:21:18Z

tests/fast/SwizzledRollbackSideband.toml failed. Unrelated.

foundationdb-ci · 2026-04-14T19:36:53Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 84cdb05
Duration 3:00:17
Result: ❌ FAILED
Error: Error while executing command: TEST_USERNAME=fdb-pr-${CODEBUILD_BUILD_NUMBER} make -C e2e foundationdb-pr-tests. Reason: exit status 2
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2026-04-14T19:50:04Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:28:33
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T20:04:41Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 84cdb05
Duration 0:43:11
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T20:37:17Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 84cdb05
Duration 1:15:46
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T20:47:40Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 84cdb05
Duration 1:26:07
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

saintstack · 2026-04-14T20:54:15Z

tests/restarting/from_7.3.0/ConfigureStorageMigrationTestRestart-1.toml Unrelated

foundationdb-ci · 2026-04-14T21:18:24Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:23:56
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T21:28:05Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 84cdb05
Duration 0:33:35
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T21:38:06Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 84cdb05
Duration 0:43:34
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T21:41:44Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 84cdb05
Duration 0:47:11
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T21:48:51Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:54:22
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T21:51:39Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:57:10
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-14T22:21:44Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 84cdb05
Duration 3:00:11
Result: ❌ FAILED
Error: Error while executing command: TEST_USERNAME=fdb-pr-${CODEBUILD_BUILD_NUMBER} make -C e2e foundationdb-pr-tests. Reason: exit status 2
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2026-04-14T23:54:45Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 84cdb05
Duration 3:00:16
Result: ❌ FAILED
Error: Error while executing command: TEST_USERNAME=fdb-pr-${CODEBUILD_BUILD_NUMBER} make -C e2e foundationdb-pr-tests. Reason: exit status 2
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2026-04-15T00:11:59Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 84cdb05
Duration 0:31:40
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-15T00:14:14Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 84cdb05
Duration 0:33:58
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-15T00:23:07Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 84cdb05
Duration 0:42:48
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-15T00:47:34Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 84cdb05
Duration 1:07:17
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-15T00:56:04Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 84cdb05
Duration 1:15:48
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-04-15T02:40:31Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 84cdb05
Duration 3:00:14
Result: ❌ FAILED
Error: Build has timed out.
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

…e#12986) Return "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Remove unused backup_parse_s3_response_failure error code. Fix const-ref parameter and add unit tests. (cherry picked from commit 7388f54)

…e#12986) Add parseErrorCodeFromS3 function that safely extracts S3 error codes from XML responses, returning "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Wire it into doRequest_impl error handling to log S3ErrorCode and detect bad requests. Add unit tests. (cherry picked from commit 7388f54)

…12997) * Harden parseErrorCodeFromS3 to never throw on non-XML responses (#12986) Add parseErrorCodeFromS3 function that safely extracts S3 error codes from XML responses, returning "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Wire it into doRequest_impl error handling to log S3ErrorCode and detect bad requests. Add unit tests. (cherry picked from commit 7388f54) * Log all S3 4xx client errors, not just 400 Widen the S3 error logging from only HTTP 400 to all 4xx client errors (403, 404, 409, etc). Remove unused badRequestCode variable. Rename trace event from S3BlobStoreBadRequest to S3BlobStoreClientError. * Fix trace * Simplify S3BlobStore request error handling (merge PR #13001) * Remove separate S3BlobStoreClientError event, use existing event --------- Co-authored-by: michael stack <stack@duboce.com>

…pple#12997) * Harden parseErrorCodeFromS3 to never throw on non-XML responses (apple#12986) Add parseErrorCodeFromS3 function that safely extracts S3 error codes from XML responses, returning "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Wire it into doRequest_impl error handling to log S3ErrorCode and detect bad requests. Add unit tests. (cherry picked from commit 7388f54) * Log all S3 4xx client errors, not just 400 Widen the S3 error logging from only HTTP 400 to all 4xx client errors (403, 404, 409, etc). Remove unused badRequestCode variable. Rename trace event from S3BlobStoreBadRequest to S3BlobStoreClientError. * Fix trace * Simplify S3BlobStore request error handling (merge PR apple#13001) * Remove separate S3BlobStoreClientError event, use existing event --------- Co-authored-by: michael stack <stack@duboce.com>

…) (#12996) Return "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Remove unused backup_parse_s3_response_failure error code. Fix const-ref parameter and add unit tests. (cherry picked from commit 7388f54)

* Harden parseErrorCodeFromS3 to never throw on non-XML responses 7.3 (#12997) - 7.3 * Harden parseErrorCodeFromS3 to never throw on non-XML responses (#12986) - main Add parseErrorCodeFromS3 function that safely extracts S3 error codes from XML responses, returning "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Wire it into doRequest_impl error handling to log S3ErrorCode and detect bad requests. Add unit tests. (cherry picked from commit 7388f54) * Log all S3 4xx client errors, not just 400 Widen the S3 error logging from only HTTP 400 to all 4xx client errors (403, 404, 409, etc). Remove unused badRequestCode variable. Rename trace event from S3BlobStoreBadRequest to S3BlobStoreClientError. * Fix trace * Simplify S3BlobStore request error handling (merge PR #13001) * Remove separate S3BlobStoreClientError event, use existing event

…pple#12997) * Harden parseErrorCodeFromS3 to never throw on non-XML responses (apple#12986) Add parseErrorCodeFromS3 function that safely extracts S3 error codes from XML responses, returning "" instead of throwing on parse failures since many HTTP error responses (502/503 HTML pages, empty bodies) are not XML. Wire it into doRequest_impl error handling to log S3ErrorCode and detect bad requests. Add unit tests. (cherry picked from commit 7388f54) * Log all S3 4xx client errors, not just 400 Widen the S3 error logging from only HTTP 400 to all 4xx client errors (403, 404, 409, etc). Remove unused badRequestCode variable. Rename trace event from S3BlobStoreBadRequest to S3BlobStoreClientError. * Fix trace * Simplify S3BlobStore request error handling (merge PR apple#13001) * Remove separate S3BlobStoreClientError event, use existing event --------- Co-authored-by: michael stack <stack@duboce.com>

saintstack requested review from Copilot and gxglass April 14, 2026 16:36

Copilot started reviewing on behalf of saintstack April 14, 2026 16:37 View session

saintstack mentioned this pull request Apr 14, 2026

Add S3ErrorCode to S3BlobStore #12979

Closed

5 tasks

Copilot AI reviewed Apr 14, 2026

View reviewed changes

gxglass approved these changes Apr 14, 2026

View reviewed changes

saintstack closed this Apr 14, 2026

saintstack reopened this Apr 14, 2026

saintstack closed this Apr 14, 2026

saintstack reopened this Apr 14, 2026

saintstack closed this Apr 14, 2026

saintstack reopened this Apr 14, 2026

saintstack closed this Apr 14, 2026

saintstack reopened this Apr 14, 2026

saintstack merged commit 7388f54 into apple:main Apr 15, 2026
10 of 11 checks passed

saintstack mentioned this pull request Apr 18, 2026

Harden parseErrorCodeFromS3 to never throw on non-XML responses #13021

Merged

Uh oh!

Conversation

saintstack commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

gxglass left a comment

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

saintstack commented Apr 14, 2026

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

saintstack commented Apr 14, 2026

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

saintstack commented Apr 14, 2026

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Apr 14, 2026

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Apr 14, 2026

saintstack commented Apr 14, 2026 •

edited

Loading