Provide a way to differentiate armeria service's internal errors

[jrhee17]

As an example, users may want to throw IllegalArgumentException from their own services.

However, they may want to return a different error code for

1. IllegalArgumentException from their own services
2. IllegalArgumentException from armeria's internal validation (i.e. missing param)

• https://github.com/line/armeria/blob/d9b6142ecc673e1690a963d7ce8920e6272d98da/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L1038

We may consider:

• Allowing users to customize the default exception returned from armeria's internal logic
  • We need to find a way to think of a way to reflect the default exception mapping to the DefaultServerErrorHandler
  • https://github.com/line/armeria/blob/d4270dac5702eb84d5790bc7bc1e9afaae528963/core/src/main/java/com/linecorp/armeria/server/DefaultServerErrorHandler.java#L65
• Also provide information on where the exception was thrown as a parameter in ServiceErrrorHandler#onServiceException
• Just wrap all exceptions with a pre-defined Armeria exception

This doesn't necessarily have to be limited to AnnotatedService, but may be also applied to GrpcService, THttpService, GraphQlService, etc.

ref: https://discord.com/channels/1087271586832318494/1087272728177942629/1156905488672379000

[my4-dev]

Let me work on this! I have two questions for this issue.

Also provide information on where the exception was thrown as a parameter in ServiceErrrorHandler#onServiceException

Is the information on where the exception was thrown included in stack trace? Do you intend to add other information?

Just wrap all exceptions with a pre-defined Armeria exception

Is it correct that it is required to implement a pre-defined Armeria exception in Armeria framework and isn't required to provide a function which makes it possible for users to decide which exception thrown exceptions are wrapped by?

[jrhee17]

Is the information on where the exception was thrown included in stack trace? Do you intend to add other information?

This was just an idea I had which turned out not to be such a good idea. The original idea I had was to just pass a parameter on where the exception was thrown from: e.g.

From

ServerErrorHandler#onServiceException(ServiceRequestContext ctx, Throwable cause)

To

ServerErrorHandler#onServiceException(ServiceRequestContext ctx, Throwable cause, boolean isInternalError)

Again, I don't think this is a good idea so please ignore it.

I think just throwing AnnotatedServiceException is enough.

Is it correct that it is required to implement a pre-defined Armeria exception in Armeria framework and isn't required to provide a function which makes it possible for users to decide which exception thrown exceptions are wrapped by?

I agree. Since our default ServerErrorHandler is checking for IllegalArgumentException, I think giving too much flexibility to the users may break behavior unintentionally. For this iteration, I think just throwing AnnotatedServiceException is enough.

Let me check with the others just to be sure of the direction before implementation begins though 😄

[jrhee17]

Sorry about the delay, how about:

• We don't try to migrate all exceptions, but just focus on request binding exceptions in annotated service and expand from there?
• We throw a RequestBindingException when a request fails to bind to a service (i.e. if there is a mandatory parameter, but the request doesn't have it)
  • RequestBindingException inherits from IllegalArgumentException to minimize breaking changes.
  • Optionally, we may also throw concrete types which inherit from RequestBindingException (i.e. MissingRequestParameterException) like spring does
    • https://github.com/spring-projects/spring-framework/tree/main/spring-web/src/main/java/org/springframework/web/bind