rsocket-java icon indicating copy to clipboard operation
rsocket-java copied to clipboard

Support returning ApplicationError with cause

Open LifeIsStrange opened this issue 2 years ago • 1 comments

Rsocket define two ApplicationError constructors, which match the standard Java Exception interface:

public final class ApplicationErrorException extends RSocketErrorException {

  private static final long serialVersionUID = 7873267740343446585L;

  /**
   * Constructs a new exception with the specified message.
   *
   * @param message the message
   */
  public ApplicationErrorException(String message) {
    this(message, null);
  }

  /**
   * Constructs a new exception with the specified message and cause.
   *
   * @param message the message
   * @param cause the cause of this exception
   */
  public ApplicationErrorException(String message, @Nullable Throwable cause) {
    super(ErrorFrameCodec.APPLICATION_ERROR, message, cause);
  }
}

from https://github.com/rsocket/rsocket-java/blob/c80b3cb6437046f3ccc79136a66299448f58c561/rsocket-core/src/main/java/io/rsocket/exceptions/ApplicationErrorException.java

My use case is trivial and extremely common, I have a Spring webflux RSocket route that return a Flux<Model> however in case where validation fail, we need to throw an error. The error inherit ExceptIon(message, cause) where cause is a Throwable Exception that contains useful data about the error (like the Exception name, error code, timestamp) which is crucial for debugging/observability and reacting to specific exceptions.

However as we can see in the code of the Exceptions file in RSocket-core:

      switch (errorCode) {
        case APPLICATION_ERROR:
          return new ApplicationErrorException(message);

from https://github.com/rsocket/rsocket-java/blob/c80b3cb6437046f3ccc79136a66299448f58c561/rsocket-core/src/main/java/io/rsocket/exceptions/Exceptions.java

When we throw an error, it will never use the ApplicationErrorException(message, cause) constructor, in fact this constructor is not called even once in the RSocket-core codebase. This crucial limitation prevent the use of RSocket in enterprise code in my opinion. Proposed Solution: Serialize not just the message but also the cause if RSocket detect an APPLICATION_ERROR. If the cause is not null, provide it in the constructor which will enable clients to consume the Error metadatas (cause)

friendly ping: @OlegDokuka @arodionov @rstoyanchev

LifeIsStrange avatar Oct 04 '21 14:10 LifeIsStrange

@LifeIsStrange thanks for raising this.

I think we need to change this so that the error details in the ERROR frame are explicitly chosen by the application. Automatically using the message of a Throwable or its cause is problematic since it is likely to reveal implementation details that aren't meant to be sent remotely.

What we can do is enhance RSocketException to accept error data in addition to the error code, separately from the Throwable message. That way the application can choose what should be on the wire as an explanation but by default we would not include a Throwable's message.

rstoyanchev avatar Mar 18 '22 10:03 rstoyanchev