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

Publisher#timeout always propagate TimeoutException #2555

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

Scottmitch
Copy link
Member

Motivation:
Publisher#timeout operator is inconsistent with Single and Completable timeout operators in that when a timeout triggers it will not ensure the timeout exception is propagated. It is possible for upstream to propagate a different exception even though the source of the error is a timeout exception.

Modifications:

  • Enhance ConcurrentTerminalSubscriber to allow for external termination and take advantage of that in Publisher#timeout.
  • Add tests for Completable and Single timeout operators to ensure existing expected behavior doesn't regress.

Motivation:
Publisher#timeout operator is inconsistent with Single and Completable
timeout operators in that when a timeout triggers it will not ensure
the timeout exception is propagated. It is possible for upstream to
propagate a different exception even though the source of the error
is a timeout exception.

Modifications:
- Enhance ConcurrentTerminalSubscriber to allow for external termination
and take advantage of that in Publisher#timeout.
- Add tests for Completable and Single timeout operators to ensure
existing expected behavior doesn't regress.
@Scottmitch
Copy link
Member Author

test failure attributed to #2245

Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Scottmitch Scottmitch merged commit de05283 into apple:main Apr 21, 2023
@Scottmitch Scottmitch deleted the pub_timeout branch April 21, 2023 02:16
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