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

Support returning ApplicationError with cause #1026

Open
LifeIsStrange opened this issue Oct 4, 2021 · 1 comment
Open

Support returning ApplicationError with cause #1026

LifeIsStrange opened this issue Oct 4, 2021 · 1 comment
Milestone

Comments

@LifeIsStrange
Copy link

LifeIsStrange commented Oct 4, 2021

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
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

@OlegDokuka OlegDokuka added this to the 1.1.2 milestone Mar 18, 2022
@rstoyanchev
Copy link
Contributor

@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 rstoyanchev modified the milestones: 1.1.2, 1.1.3 Mar 28, 2022
@OlegDokuka OlegDokuka modified the milestones: 1.1.3, 1.1.4 Sep 14, 2022
@OlegDokuka OlegDokuka modified the milestones: 1.1.4, 1.2.0-M1 Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants