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

Async API: Fail-fast or Future-transported exceptions? #119

Closed
mp911de opened this issue Aug 18, 2015 · 2 comments
Closed

Async API: Fail-fast or Future-transported exceptions? #119

mp911de opened this issue Aug 18, 2015 · 2 comments
Labels
for: team-attention An issue we need to discuss as a team to make progress

Comments

@mp911de
Copy link
Collaborator

mp911de commented Aug 18, 2015

The synchronous and reactive API are consistent in handling exceptions:

  • sync calls throw instances of RedisExceptions on invocations
  • calls to Observable<T> transport exceptions within on onError

The asynchronous API sometimes throws exceptions on invoking a Redis command and sometimes exceptions are carried within the RedisFuture (CompletionStage).

The question here is, whether to shift all exceptions to the future or keep it as-is.

Examples for exception cases:

  • Redis connection is disconnected, and auto-reconnect is disabled: future-transported RedisException: Connection is in a disconnected state and reconnects disabled. Commands are not accepted.
  • Connection is closed (by user): command invocation throws RedisException: Connection is closed
  • Invalid command args (nulls, serialization issues): command invocation throws IllegalArgumentException
  • Request queue size exceeded: command invocation throws RedisException: Request queue size exceeded
  • Exceptions while request processing (Exceptions in the transport, IOExceptions, SSL Handshake errors, ... [except disconnections]): future-transported exceptions
  • Exceptions while response processing (Deserialization, Exceptions in the transport): future-transported exceptions
@mp911de mp911de added the for: team-attention An issue we need to discuss as a team to make progress label Aug 18, 2015
mp911de added a commit that referenced this issue Aug 19, 2015
Throw RedisException instead of future-transport on disconnected state when reconnect is disabled.
mp911de added a commit that referenced this issue Aug 19, 2015
Throw RedisException instead of future-transport on disconnected state when reconnect is disabled.
mp911de added a commit that referenced this issue Aug 19, 2015
Throw RedisException instead of future-transport on disconnected state when reconnect is disabled.
@mp911de
Copy link
Collaborator Author

mp911de commented Aug 19, 2015

Changed "Redis connection is disconnected, and auto-reconnect is disabled" exception to fail-fast since that exception came in in 3.3/4.0 and both versions are not released now.

@mp911de
Copy link
Collaborator Author

mp911de commented Dec 11, 2015

Closing this ticket. The strategy is fail-fast over Future-transported exceptions.

@mp911de mp911de closed this as completed Dec 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress
Projects
None yet
Development

No branches or pull requests

1 participant