Skip to content

Conversation

RanVaknin
Copy link
Contributor

Motivation and Context

This test had two separate race conditions:

  1. the Transformer's exceptionOccurred function and the Subscriber's failed() method both race to complete the completeablefuture exceptionally resulting in:
java.lang.AssertionError: 
Expecting a cause with type:
  "java.lang.RuntimeException"
and message:
  "oops"
but type was:
  "java.nio.channels.ClosedChannelException"
and message was:
  null.

This can be solved by making the assertion more relaxed. This test is meant to test the delete file behavior, and not the exception handling. I believe this exception assertion that was added in the stubException helper function was to enforce the contract (an exception is propagated when the file transformer is terminated). This race condition caused that the specific exception could not be guaranteed.

  1. The FileSubscriber's preformWrite finished writing before the Transformer's exceptionOccurred handler was able to complete the future exceptionally:
java.lang.AssertionError: 
Expecting
  <CompletableFuture[Completed: "foobar"]>
to have failed within 1L SECONDS.
Be aware that the state of the future in this message might not reflect the one at the time when the assertion was performed as it is evaluated later on

The fix replaces the original SdkPublisher that emits data with Flowable.just(content, content) with an idle publisher that emits data but never completes.
The original publisher would emit and immediately complete, creating a race between normal completion and exception handling. The idle publisher keeps the stream open, while the exception handling path has time to execute before completion.

Testing

  1. Created a @RepeatedTest wrapper with a 1000 executions to check for failure rates. The test was failing 0.4-1.2% of tests. After the fix I see a 0% failure rate.

  2. Added logging to each handler of the FileSubscriber and the Transformer to observe the race conditions. For example:

transformer calling onSubscribe
Transformer's exceptionOccurred start block
Transformer's exceptionOccurred: closing the channel
Transformer's exceptionOccurred: cleanup
transformer completed method is called
File Subscriber failed, future.completeExceptionally(exc) is called: java.nio.channels.ClosedChannelException
Transofmer exceptionOccured completes the future exceptionally

java.lang.AssertionError: 
Expecting a cause with type:
  "java.lang.RuntimeException"
and message:
  "oops"
but type was:
  "java.nio.channels.ClosedChannelException"
and message was:
  null.

@RanVaknin RanVaknin requested a review from a team as a code owner March 26, 2025 23:00
@RanVaknin RanVaknin force-pushed the rvaknin/fix-integ-FileAsyncResponseTransformerTest branch from 7a2b1e4 to 3f55b2e Compare March 27, 2025 18:36
Copy link

@RanVaknin RanVaknin added this pull request to the merge queue Mar 31, 2025
Merged via the queue into master with commit 1ec1c12 Mar 31, 2025
19 checks passed
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.

2 participants