Skip to content

Conversation

RanVaknin
Copy link
Contributor

@RanVaknin RanVaknin commented Sep 22, 2025

The mpuDefaultSplitImpl_partsFailOfRetryableError_shouldFail test intermittently fails in CI environments following PR #6368 which replaced the direct SplittingPublisher constructor with a builder pattern, which likely changed the threading or timing behavior of when the NonRetryableException gets thrown versus when WireMock processes and counts the HTTP requests.

This seems to cause a race condition in the test where sometimes the test sees the expected single request and sometimes it sees multiple requests before the exception occurs:

[ERROR] software.amazon.awssdk.services.s3.internal.multipart.S3MultipartClientPutObjectWiremockTest.mpuDefaultSplitImpl_partsFailOfRetryableError_shouldFail(String, Long, ResponseDefinitionBuilder)[1] -- Time elapsed: 0.544 s <<< FAILURE!
com.github.tomakehurst.wiremock.client.VerificationException:
Expected exactly 1 requests matching the following pattern but received 2:

or

[ERROR] software.amazon.awssdk.services.s3.internal.multipart.S3MultipartClientPutObjectWiremockTest.mpuDefaultSplitImpl_partsFailOfRetryableError_shouldFail(String, Long, ResponseDefinitionBuilder)[2] -- Time elapsed: 0.151 s <<< FAILURE
java.lang.AssertionError:
Expecting throwable message:

"software.amazon.awssdk.core.exception.NonRetryableException: Multiple subscriptions detected. This could happen due to a retry attempt. The AsyncRequestBody implementation provided does not support splitting to retryable/resubscribable AsyncRequestBody. If you need retry capability or multiple subscriptions, consider using BufferedSplittableAsyncRequestBody to wrap your AsyncRequestBody. (SDK Attempt Count: 2)"
to contain:
"A retry was attempted, but"
but did not.

When testing this locally, I ran a parameterizedTest with 1000 iterations and saw 100% success rate even when running in parallel. This is probably failing in our CI build for some environment specific reasons. As first low effort measure, I added a simple retryer that will retry each parameterized test 3 times before failing.

@RanVaknin RanVaknin requested a review from a team as a code owner September 22, 2025 07:06
@RanVaknin RanVaknin added changelog-not-required Indicate changelog entry is not required for a specific PR no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required labels Sep 22, 2025
Copy link

return Optional.ofNullable(contentLength);
for (int i = 0; i < 3; i++) {
try {
stubUploadPartFailsInitialAttemptSucceedsUponRetryCalls(responseDefinitionBuilder);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe consider put this line out of the for loop to prevent multiple stubs. I'm not sure if this will just overwrite the previous stub or cause multiple stub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-not-required Indicate changelog entry is not required for a specific PR no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants