Generator-jhipster: Use Zalando Problem for all json responses

[ecostanzi]

Overview of the issue

I think it could be useful to choose a unique json format to handle API errors.

The exception handling uses zalando problem through the ExceptionTranslator.

{
    "type": "http://www.jhipster.tech/problem/contraint-violation",
    "title": "Method argument not valid",
    "status": 400,
    "message": "error.validation",
    "fieldErrors": [
        {
            "objectName": "todoListDTO",
            "field": "name",
            "message": "NotNull"
        }
    ]
}

The EntityResource controllers return the error details in the HTTP headers, e.g.

x-jtodoapp-error →A new todoList cannot already have an ID

the AccountResource has its own logic, for example for wrong credentials

{
    "AuthenticationException": "Bad credentials"
}

the Http401UnauthorizedEntryPoint returns the spring json default format

{
    "timestamp": "2017-09-25T10:06:18.672+0000",
    "status": 401,
    "error": "Unauthorized",
    "message": "Access Denied",
    "path": "/api/todo-items"
}

I'm wondering if there is some reason why the exception translator and zalando problem (or the ErrorVM before it) aren't use globally.

Motivation for or Use Case

Most of the APIs we generate with jhipster are exposed to external applications (both web and mobile). We noticed that even documenting these different behaviours it is still quite confusing for those who have to interact with the APIs.

Suggest a Fix

Exceptions could be used in all the generated controllers and in the accountResource, so that all the errors are handled by the ExceptionTranslator.
The Http401UnauthorizedEntryPoint could then use the object mapper to serialize a Problem object in the response body.

JHipster Version(s)

4.8.2

• [x] Checking this box is mandatory (this is just to show you read everything)

[cbornet]

I'm precisely working on this :smile:
@henri-tremblay

[ecostanzi]

Good! :) Looking forward for it

[henri-tremblay]

@cbornet Good, I'll let you work :-) I'm currently fighting with a bug in Problem serialization (sometimes, the status serializer isn't found at deserialization. I don't know why). Now #6409

[henri-tremblay]

One thing I have noticed before I forgot. If it can be of any use.

ConstraintViolationProblem is not serialized directly. It is transformed into a DefaultProblem. Which means we have one layer of indirection to get to the violations (parameters.fieldErrors instead of violations). My question is why not returning ConstraintViolationProblem? I don't think the ConstraintViolationProblemModule is used in any way because of that.

[henri-tremblay]

Oh... I know there was 2 things. So, the second thing I noticed is that a ThrowableProblem contains a stack trace which is then serialized. For security reasons, we avoid featuring internal implementations. But now it's all there in front of us. Is there a way to fix that? Maybe the ExceptionTranslator could squeeze the stacktrace?

[atomfrede]

The Jackson problem module has a property with stacktrace. We usually enable it on our dev environment and disable it on all others. Think we could do it too.

[cbornet]

The stacktrace should be disabled by default . @henri-tremblay do you have stacktraces in your responses ?

[cbornet]

My question is why not returning ConstraintViolationProblem ?

Because it doesn't include the objectName field that we use. I think it could be added by PR to zalando's project.

[henri-tremblay]

@cbornet Ok. Again something I don't get it seems. In integration tests, the stack trace is there. But indeed, it's not when calling the API on the running app. So all good (but I don't know why...)

About objectName, they used to have it but removed it on purpose. See https://github.com/zalando/problem-spring-web/issues/86

[cbornet]

The concerns in https://github.com/zalando/problem-spring-web/issues/86 seem legit.
That said if we want to remove FieldErrorVM and use violations directly, we also need the code that we use for translation (based on the user selected language, not the browser one). Maybe we can PR for that.

[cbornet]

Fixed by #6459 and #6411