[RFC] Implementing externally documented error codes across the Sling APIs

[José Correia <me...@gmail.com>]

I would like to open the discussion on creating a standard for well-defined
and externally documented error codes in our APIs.

This simple approach is a very good way of keeping consistency on the
errors thrown by the API and improving error documentation, while also
simplifying the way that exceptions
that happen in lower-level modules of the system are handled by higher
modules such as servlet endpoints.

Stripe has a good example of good error code documentation:
https://stripe.com/docs/error-codes

*Pros:*

   1. A single place of change for API error descriptions.
   2. We can change the error descriptions without changing the error code,
   which improves logging in the long term.
   3. Can be extended to add links for the error code external
   documentation. This could improve the API responses, providing more
   feedback on errors.
   4. Easier propagation of exception expected outcomes:
      1. the expected description, error code, and HTTP status are
      propagated easily inside the exceptions
      2. This allows for the transport layer to be completely abstracted of
      exceptions in lower modules, and simply act according to the error code
      definition.


A PR was created implementing an initial approach in the Distribution Core
bundle:
https://github.com/apache/sling-org-apache-sling-distribution-core/pull/60

Comments are welcome!

Short example

*Error code definition:*

public enum ErrorCode {
    /**     * A generic unexpected error in the system.     * Should
result in an INTERNAL_SERVER_ERROR HTTP status code.     */
    UNKNOWN_ERROR("unknown", "An unexpected error has occurred.", 500),
    /**     * Error that happens when the client tries to distribute a
package that exceeds the     * configured package size limit.     *
Should result in a BAD_REQUEST HTTP status code.     */
    DISTRIBUTION_PACKAGE_SIZE_LIMIT_EXCEEDED("package_limit_exceeded",
"Package has exceeded the" +
            " limit bytes size.", 400);}


*Exceptions include this type as an additional property:*

public class DocumentedException extends Exception {

    private ErrorCode errorCode;}

*Transport layer handles exception in an abstract way:*

try {
  // execute logic} catch (Exception e) {
  ErrorCode error = e.getErrorCode();
  logger.error(error.getCode(), error.getDescription());

  httpStatusCode = e.getHttpStatusCode();
  -> return httpStatusCode and JSON containing description and code.}

Re: [RFC] Implementing externally documented error codes across the Sling APIs

[Eric Norman <en...@apache.org>]

Hi,

Having a better way to correlate errors to additional information seems
like it could be useful.

I have a couple of questions to get discussion going:

   1. Would this be per-bundle error codes and per-bundle documentation? Or
   were you thinking that there would be some sharing or common error codes
   that would need to be provided by some shared bundle?  Can we ensure unique
   error codes without some sort of namespace prefix on the errorCode?
   2. Were you thinking that documentation would get auto-generated from
   some tooling via the details in the enumeration source code or manually
   maintained?
   3. Since some of these seem to be errors that make it all the way back
   to the end user (and not just logged) then would it make sense to have some
   i18n support so the text can be translated to other languages?
   4. It may not be possible for the thrower of the exception to know what
   the http status code should be, perhaps that is a decision that is better
   left to the code that catches the exception?
   5. Do you have any thoughts on how this would impact the already
   existing exceptions and error handling?

Regards,
Eric

On Fri, Jun 3, 2022 at 3:05 AM José Correia <me...@gmail.com> wrote:

> I would like to open the discussion on creating a standard for well-defined
> and externally documented error codes in our APIs.
>
> This simple approach is a very good way of keeping consistency on the
> errors thrown by the API and improving error documentation, while also
> simplifying the way that exceptions
> that happen in lower-level modules of the system are handled by higher
> modules such as servlet endpoints.
>
> Stripe has a good example of good error code documentation:
> https://stripe.com/docs/error-codes
>
> *Pros:*
>
>    1. A single place of change for API error descriptions.
>    2. We can change the error descriptions without changing the error code,
>    which improves logging in the long term.
>    3. Can be extended to add links for the error code external
>    documentation. This could improve the API responses, providing more
>    feedback on errors.
>    4. Easier propagation of exception expected outcomes:
>       1. the expected description, error code, and HTTP status are
>       propagated easily inside the exceptions
>       2. This allows for the transport layer to be completely abstracted of
>       exceptions in lower modules, and simply act according to the error
> code
>       definition.
>
>
> A PR was created implementing an initial approach in the Distribution Core
> bundle:
> https://github.com/apache/sling-org-apache-sling-distribution-core/pull/60
>
> Comments are welcome!
>
> Short example
>
> *Error code definition:*
>
> public enum ErrorCode {
>     /**     * A generic unexpected error in the system.     * Should
> result in an INTERNAL_SERVER_ERROR HTTP status code.     */
>     UNKNOWN_ERROR("unknown", "An unexpected error has occurred.", 500),
>     /**     * Error that happens when the client tries to distribute a
> package that exceeds the     * configured package size limit.     *
> Should result in a BAD_REQUEST HTTP status code.     */
>     DISTRIBUTION_PACKAGE_SIZE_LIMIT_EXCEEDED("package_limit_exceeded",
> "Package has exceeded the" +
>             " limit bytes size.", 400);}
>
>
> *Exceptions include this type as an additional property:*
>
> public class DocumentedException extends Exception {
>
>     private ErrorCode errorCode;}
>
> *Transport layer handles exception in an abstract way:*
>
> try {
>   // execute logic} catch (Exception e) {
>   ErrorCode error = e.getErrorCode();
>   logger.error(error.getCode(), error.getDescription());
>
>   httpStatusCode = e.getHttpStatusCode();
>   -> return httpStatusCode and JSON containing description and code.}
>