Custom data for all problem exceptions

[Torsph]

Hi!

I want to add different values to all exceptions. For example zipkin traceId.

I have added:

@Configuration
class SpringDataRestConfiguration extends RepositoryRestConfigurerAdapter {

  @Override
  public void configureJacksonObjectMapper(ObjectMapper objectMapper) {
    objectMapper
        .registerModule(new ProblemModule())
        .registerModule(new ConstraintViolationProblemModule());
  }
}

but when I try to override methods in:

@ControllerAdvice
class GlobalExceptionHandler implements ProblemHandling {
}

I end up with getting the full problem serialized cause, stackTrace, etc.

I must have misunderstood anything about the customization. But do you have any good way to add custom data and how to do the customization?

Thanks!

[Torsph]

Changed to:

@Configuration
public class JacksonConfiguration {

  @Bean
  public ProblemModule problemModule() {
    return new ProblemModule();
  }

  @Bean
  public ConstraintViolationProblemModule constraintViolationProblemModule() {
    return new ConstraintViolationProblemModule();
  }
}

and now it serialize correctly.

And I have added the custom variable like this:

  @Override
  public ResponseEntity<Problem> create(final Throwable throwable, final Problem problem,
      final NativeWebRequest request, final HttpHeaders headers) {
    String traceId = "some-trace-id"

    Problem p = Problem.builder()
        .withDetail(problem.getDetail())
        .withInstance(problem.getInstance())
        .withStatus(problem.getStatus())
        .withTitle(problem.getTitle())
        .withType(problem.getType())
        .with("traceId", traceId).build();

    return ProblemHandling.super.create(throwable, p, request, headers);
  }

Is there any cleaner way of doing it?

[whiskeysierra]

Your proposal should work, but it will loose all other custom properties of that problem. I can't think of a clean solution right now. Maybe some magic with Jackson?

[Torsph]

Thanks for the answer @whiskeysierra..

I added problem.getParameters().forEach(builder::with); at the end so it should not loose any of the custom props.

I have been looking through the code for a better option then recreating the Problem. The only place I found was the ProblemBuilder prepare(final Throwable throwable, final StatusType status, final URI type), but that is not called on all Exceptions.

[whiskeysierra]

I added problem.getParameters().forEach(builder::with); at the end so it should not loose any of the custom props.

Unless it's a custom sub-type of Problem that introduces fields/properties.

I have been looking through the code for a better option then recreating the Problem. The only place I found was the ProblemBuilder prepare(final Throwable throwable, final StatusType status, final URI type), but that is not called on all Exceptions.

That's true. If the original exception is already a problem, then it's not used.

Maybe this method could be useful for you: https://github.com/zalando/problem-spring-web/blob/master/src/main/java/org/zalando/problem/spring/web/advice/AdviceTrait.java#L290

[Torsph]

That method could work.

  @Override public ResponseEntity<Problem> process(ResponseEntity<Problem> entity) {
    String traceId = Optional.ofNullable(spanAccessor.getCurrentSpan()).map(Span::traceIdString).orElse("");
    entity.getBody().getParameters().put("traceId", traceId);
    return entity;
  }

However, I need to do some other changes to the problem when getting the title and the details.

  private Problem describingProblem(Throwable throwable, Problem problem) {
    Throwable rootCause = getRootCause(throwable);
    if (rootCause instanceof Problem) {
      return (Problem) rootCause;
    } else {
      return problem;
    }
  }

or else it will get something that looks like a stacktrace in the response. There are some libs that are throwing parts of their stacktrace inside the details.

[cbornet]

@whiskeysierra I think a copy method to construct a ProblemBuilder from a Problem would be very useful

[whiskeysierra]

I think a copy method to construct a ProblemBuilder from a Problem would be very useful

That would not be easy to do for users that choose to implement Problem directly.

[whiskeysierra]

What about this trick? https://stackoverflow.com/questions/14714328/jackson-how-to-add-custom-property-to-the-json-without-modifying-the-pojo

[whiskeysierra]

I'm closing this, as I believe that this should rather be achieved using Jackson. @Torsph @cbornet Feel free to re-open/object.

[dominikmontada]

@whiskeysierra I just ran into this problem myself. I am also trying to add the trace id to all exception and while it is easily doable for the default errors by overriding the prepare method, it is difficult to maintain for ConstraintViolationProblem and other custom problems as they might not use the builder from prepare. I also don't think that using Jackson to achieve this is the right way to go. It seems very counter-intuitive and complicated for what I believe could be a common use case.

You already provide a method process to do some post processing on the final ResponseEntity. Why not make Problem more customizable after it has already been created (e.g. a method add to add custom fields). That way you could simply use the process method to achieve what you want.

At a later stage it might even make sense to revise the class tree to provide a common base class that custom problems can extend from. I only looked at the source code briefly and certainly haven't understood all of it fully yet, but the current structure (ConstraintViolationProblem not extending DefaultProblem) seems to make this overly complicated and is not very intuitive at first.

But anyway, as I said: having this feature in some form without having to use some voodoo magic would be greatly appreciated!

[whiskeysierra]

ConstraintViolationProblem and other custom problems as they might not use the builder from prepare

Problem is an interface and meant to be immutable. Users of the problem library are encourage to implement this interface directly without using any of the default implementations. Based on that using jackson to manipulate the resulting JSON is actually the cleanest way to do it. Personally I'd say an HTTP header is even cleaner and less intrusive to implement.

[cbornet]

Maybe adding a toProblemBuilder() method in Problem would help (forcing implementations to implement it) ?

[cbornet]

Or another solution would be to require that getParameters return all Extension members (ie. remove the Implementations can choose to ignore this in favor of concrete typed fields. sentence) which would be more coherent with Problem being an interface.

[dominikmontada]

Personally I'd say an HTTP header is even cleaner and less intrusive to implement.

Unfortunately our specification says that this needs to be in the JSON itself, not the Header. Otherwise I would agree.

Users of the problem library are encouraged to implement this interface directly without using any of the default implementations.

If that is the case why provide a default implementation in the first place? The default implementation is already pretty good and does what I want and expect it to do. It seems redundant to having to reinvent the wheel (or more-or-less copy paste it) when I just want to add a custom field. I get that it seems possible to do with Jackson, but looking at the Stackoverflow thread that you shared above it looks more complicated that this problem ought to be. I shouldn't have to implement a new serialization module for something this simple.

Maybe adding a toProblemBuilder() method in Problem would help (forcing implementations to implement it) ?

Or another solution would be to require that getParameters return all Extension members (ie. remove the Implementations can choose to ignore this in favor of concrete typed fields. sentence) which would be more coherent with Problem being an interface.

Both sound like a reasonable way to achieve this. If @whiskeysierra agrees and @cbornet shares his idea on how this would be used in practice, I could try to implement this in a pull request if you need me to.

[AlexandreCassagne]

@dominikmontada Have you implemented this/are you willing to share a pull request? I don't know what the team's thoughts are on this are but I agree that the current approach of creating a serialisation module is convoluted.

[nazeem-soeltan]

That method could work.

  @Override public ResponseEntity<Problem> process(ResponseEntity<Problem> entity) {
    String traceId = Optional.ofNullable(spanAccessor.getCurrentSpan()).map(Span::traceIdString).orElse("");
    entity.getBody().getParameters().put("traceId", traceId);
    return entity;
  }

However, I need to do some other changes to the problem when getting the title and the details.

  private Problem describingProblem(Throwable throwable, Problem problem) {
    Throwable rootCause = getRootCause(throwable);
    if (rootCause instanceof Problem) {
      return (Problem) rootCause;
    } else {
      return problem;
    }
  }

or else it will get something that looks like a stacktrace in the response. There are some libs that are throwing parts of their stacktrace inside the details.

Unfortunately the first method doesn't work, due to an unmodifiable collection.

[cervenf]

Hello, did you find any solution to add a "global" parameter to all problems? I would like to add the service name and UUID to all problems. Thank you.

[cervenf]

Hello, I checked jhipster generator https://start.jhipster.tech/generate-application because they are using problems too. Seems that they use process() method too and they "clone" the problem and add more parameters like this:

 ProblemBuilder builder = Problem
            .builder()
            .withType(Problem.DEFAULT_TYPE.equals(problem.getType()) ? ErrorConstants.DEFAULT_TYPE : problem.getType())
            .withStatus(problem.getStatus())
            .withTitle(problem.getTitle())
            .with(PATH_KEY, requestUri);

.... **problem.getParameters().forEach(builder::with);** .... full method:

` @Override public ResponseEntity process(@Nullable ResponseEntity entity, NativeWebRequest request) { if (entity == null) { return null; } Problem problem = entity.getBody(); if (!(problem instanceof ConstraintViolationProblem || problem instanceof DefaultProblem)) { return entity; }

    HttpServletRequest nativeRequest = request.getNativeRequest(HttpServletRequest.class);
    String requestUri = nativeRequest != null ? nativeRequest.getRequestURI() : StringUtils.EMPTY;
    ProblemBuilder builder = Problem
        .builder()
        .withType(Problem.DEFAULT_TYPE.equals(problem.getType()) ? ErrorConstants.DEFAULT_TYPE : problem.getType())
        .withStatus(problem.getStatus())
        .withTitle(problem.getTitle())
        .with(PATH_KEY, requestUri);

    if (problem instanceof ConstraintViolationProblem) {
        builder
            .with(VIOLATIONS_KEY, ((ConstraintViolationProblem) problem).getViolations())
            .with(MESSAGE_KEY, ErrorConstants.ERR_VALIDATION);
    } else {
        builder.withCause(((DefaultProblem) problem).getCause()).withDetail(problem.getDetail()).withInstance(problem.getInstance());
        problem.getParameters().forEach(builder::with);
        if (!problem.getParameters().containsKey(MESSAGE_KEY) && problem.getStatus() != null) {
            builder.with(MESSAGE_KEY, "error.http." + problem.getStatus().getStatusCode());
        }
    }
    return new ResponseEntity<>(builder.build(), entity.getHeaders(), entity.getStatusCode());
}`

[whiskeysierra]

The downside with that approach is that you loose any support for custom problem types.

[cervenf]

aha you mean, e.g. in this example: https://github.com/zalando/problem#custom-problems you will lose "product" property? Is there then any better way please?

[whiskeysierra]

Exactly. No, there is no good workaround to me knowledge at least. Custom JSON post-processor using Jackson + Spring, maybe. But it won't be nice either.

[JohannesPertl]

I think this should be re-opened. The suggested solutions are only working partially and I think using Jackson is not great either for just adding a field to all problems

[Bennett-Lynch]

+1 for still wanting support here. Would like to model problems as concrete types but still have the flexibility to enrich them with extra parameters in a generic fashion (traceId is a good example, where you would typically inject this with some type of interceptor).