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

refactor(engine-rest): Ensure Proper Exception Code Assignment in ExceptionHandlerHelper #4600

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yhao3
Copy link

@yhao3 yhao3 commented Sep 13, 2024

Background

While implementing custom exception handling for my Camunda REST API, I used the ExceptionHandlerHelper class from the engine-rest module to ensure that the behavior of error handling is consistent with the built-in Camunda REST API error handling. The custom exception handler code is as follows:

@ControllerAdvice
public class CustomExceptionHandler extends ResponseEntityExceptionHandler {

    protected static final ExceptionLogger LOGGER = ExceptionLogger.REST_LOGGER;
    private static final ExceptionHandlerHelper DELEGATE = ExceptionHandlerHelper.getInstance();

    @ExceptionHandler
    public ResponseEntity<Object> handleAnyException(Throwable ex, NativeWebRequest request) {
         ExceptionDto exceptionDto = DELEGATE.fromException(ex);
         return handleExceptionInternal((Exception) ex, exceptionDto, buildHeaders(ex), HttpStatusCode.valueOf(DELEGATE.getStatus(ex).getStatusCode()), request);
    }

    @Nullable
    @Override
    protected ResponseEntity<Object> handleExceptionInternal(
            Exception ex,
            @Nullable Object body,
            HttpHeaders headers,
            HttpStatusCode statusCode,
            WebRequest request
    ) {
        LOGGER.log(ex);
        if (body == null) {
            return super.handleExceptionInternal(ex, DELEGATE.fromException(ex), headers, statusCode, request);
        }
        return super.handleExceptionInternal(ex, body, headers, statusCode, request);
    }

    private HttpHeaders buildHeaders(Throwable err) {
        // ...
    }

}

However, the code field value in the response body (ExceptionDto) was always null, resulting in outputs like the following:

{
    "type": "AuthorizationException",
    "message": "The user with id 'tom' does not have 'CREATE' permission on resource 'Deployment'.",
    "code": null,
    "userId": "tom",
    "resourceName": "Deployment",
    "resourceId": null,
    "permissionName": "CREATE",
    "missingAuthorizations": [
        {
            "permissionName": "CREATE",
            "resourceName": "Deployment",
            "resourceId": null
        }
    ]
}

Changes Overview

This PR resolves an issue where the code field in ExceptionDto was always null. The following adjustments were made:

  1. Refactored fromException Method: Ensured the provideExceptionCode method is called for all exception types, correctly assigning the exception code.
  2. Removed Redundant Code: Eliminated the redundant provideExceptionCode call in the getResponse method to avoid unnecessary processing.

@yanavasileva
Copy link
Member

@yhao3, thank you for reaching out to us with this.

We will have look at it in the next weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants