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

Jersey doesn't allow to override a built-in ValidationExceptionMapper #3425

Closed
jerseyrobot opened this issue Aug 11, 2016 · 41 comments
Closed

Comments

@jerseyrobot
Copy link
Contributor

In attachments you can see a project which uses: Jersey, Weld, Jetty and jersey bean validation module.

The problem is, when I've upgraded Jersey from 2.14 to 2.23.1 my custom exception mapper:

@Provider
public class ValidationExceptionMapper implements
        ExceptionMapper<ValidationException> {

    @Override
    public Response toResponse(ValidationException exception) {
        final ResponseEntity resp = ResponseEntity.builder()
.message("Hurray, my custom ValidationExceptionMapper was called!")
.build();

        return Response.status(Status.BAD_REQUEST).entity(resp).build();
    }
}

is no longer called - Jersey uses the built-in exception mapper for this exception: org.glassfish.jersey.server.validation.internal.ValidationExceptionMapper and unfortunately I cannot override it.

The problem is strictly related to the Weld integration. Because without dependency to Weld, my custom exception mapper has a higher priority than the one provided by jersey-bean-validation module.

Environment

Windows 7/ Ubuntu 16.04
Oracle JDK 1.7.0_75
Jetty 9.2.13.v20150730
Any Jersey version above 2.14

Affected Versions

[2.23.1]

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
Reported by gdemecki

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
gdemecki said:
Known workarounds:

  • Downgrade to Jersey 2.14
  • Remove Weld
  • Remove jersey-bean-validation module
  • Use HK2 to override the problematic mapper:```
    register(new AbstractBinder() {
    @OverRide
    protected void configure() {
    bind(my.custom.ValidationExceptionMapper.class).to(ExceptionMapper.class)
    .in(Singleton.class);
    }
    });
    
    

BTW: How can I attach a sample reproducer app? Funny, I don't have permissions ^^

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
@pavelbucek said:
link to a public github repo is best way how to do that, java.net disabled attachments long time ago..

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
gdemecki said:
Thanks Pavel,
example app is available on the GitHub.

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
@mpotociar said:
I have tried to run your example, but I can see some WELD scanning-related exceptions when I invoke mvn clean package jetty:run from my command line.
Please fix this problem in your reproducer.

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
@mpotociar said:
This is the error that I see:
WELD-ENV-000033: Invalid bean archive scanning result - found multiple results with the same reference

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
gdemecki said:
Thanks Marek, indeed reproducer isn't perfect - I must admit that I've launched it only via the com.xyz.AppRunner.main() from my IDE. Please use it instead of jetty-maven-plugin, if it isn't a problem.

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
heinbloed said:
We're trying to migrate from GlassFish 4.1 (Jersey 2.10.4) to Payara 4.1.1.164 (Jersey 2.22.2) and are affected by the same problem.

For some reason I don't know, we hadn't implemented the interface but extended the Jersey implementation: @Provider public class ValidationExceptionHandler extends ValidationExceptionMapper. This is why I also noticed that that class is final now. Changing this to @Provider public class ValidationExceptionHandler implements ExceptionMapper still works in GF 4.1 but no longer in Payara 4.1.1.164, i.e. toResponse(...) is never called.

Since Payara bundles all of the modules named by gdemecki in his workaround suggestions (and we also use all of them), none of them would be possible for us.

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
heinbloed said:
For anyone else needing a quick fix for this, I managed to have my class implementing ExceptionMapper get called again by modifying org.glassfish.jersey.server.validation.internal.ValidationBinder.configure() and commenting out the two lines

bind(ValidationExceptionMapper.class).to(ExceptionMapper.class).in(Singleton.class);

and

bind(ValidationErrorMessageBodyWriter.class).to(MessageBodyWriter.class).in(Singleton.class);

I didn't really analyze the source code for that modification, I just noticed that those two lines were added in one of the versions > 2.14. Maybe this helps someone knowledgable in the code to find a real fix.

@jerseyrobot
Copy link
Contributor Author

@glassfishrobot Commented
This issue was imported from java.net JIRA JERSEY-3153

@jerseyrobot
Copy link
Contributor Author

@deratzmann Commented
Hey guys. I'm running also into this Problem. Is there any progress so far?

@jerseyrobot
Copy link
Contributor Author

@zak905 Commented
same here. this is annoying.

@jerseyrobot
Copy link
Contributor Author

@zak905 Commented
I created an Exception mapper with same name as the one provided by jersey-beans-validation and gave it priority, maybe it can work without priority

@Priority(1)
public class ValidationExceptionMapper implements javax.ws.rs.ext.ExceptionMapper<ValidationException> { 

it works !

@jerseyrobot
Copy link
Contributor Author

@deratzmann Commented
It doesn't work for me...Is somebody at jersey having a look on that bug?

import javax.validation.ValidationException;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
@priority(value = 1)
@Provider
public class ValidationExceptionMapper implements ExceptionMapper {
@OverRide
public Response toResponse(ValidationException exception) {
return Response.status(Response.Status.BAD_REQUEST).entity("message:" + exception.getMessage()).build();
}
}

@jerseyrobot
Copy link
Contributor Author

@zak905 Commented
did you try with type parameter javax.ws.rs.ext.ExceptionMapper<ValidationException> ?

@jerseyrobot
Copy link
Contributor Author

@zak905 Commented
I confirm it works without priority, @Provider is enough, I think the important thing is that the type parameter should be ValidationException

@jerseyrobot
Copy link
Contributor Author

@deratzmann Commented
Which server do you use? I am using payara full (jersey is included) profile.

@jerseyrobot
Copy link
Contributor Author

@zak905 Commented
Tomcat, Jersey 2.26.

@jerseyrobot
Copy link
Contributor Author

@deratzmann Commented
Hello.

This was finally my fix:
`
import java.util.Set;
import javax.annotation.Priority;
import javax.validation.ConstraintViolation;
import javax.validation.ConstraintViolationException;
import javax.ws.rs.Priorities;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;

@priority(Priorities.USER)
@Provider
public class ValidationExceptionMapper implements ExceptionMapper {

@Override
public Response toResponse(ConstraintViolationException exc) {
    StringBuilder message = new StringBuilder();
    Set<ConstraintViolation<?>> violations = exc.getConstraintViolations();
    if (violations != null) {
        violations.forEach((violation) -> {
            message.append(violation.getMessage()).append(".");
        });
    }
    return Response.status(Response.Status.BAD_REQUEST).header("message", message.toString()).build();
}

}
`
It seems to be that "ConstraintViolationException" must be inserted in the generic expression.

@jerseyrobot
Copy link
Contributor Author

@juanmbellini
Copy link

In my case I use a ThrowableMapper which, in turn, uses an error handler which is shared across several frameworks (for example, spring security, which does not run inside jersey).
In this case, I would like to avoid Jersey registering the builtin ValidationExceptionMapper. Would be great to be able to register the BV feature, but only for performing the validation process, allowing me to handle the situation with my mapper.

@bric3
Copy link

bric3 commented Jun 17, 2019

I noticed that registering an instance of custom exception, make this instance actually selected to map the exception. Suppose there's

package x.y.z;

class ValidationExceptionMapper implements implements ExceptionMapper<ValidationException> {
}
var vem = new ValidationExceptionMapper();
register(vem, ExceptionMapper.class); // this works, the custom mapper is used
register(x.y.z.ValidationExceptionMapper.class, ExceptionMapper.class); // this does not

Which is handy anyway as this mapper is managed by Spring anyway. This had me confused, as since Jersey 2.26, spring integration works by registering endpoint class of the spring bean directly (as described here : #3700)

@ktalebian
Copy link

Have there been any updates on this? I still cannot get this to work. I've tried to use HK2 to bind my custom exception mapper class to ExceptionMapper.class and that also does not work.

@zak905
Copy link

zak905 commented Dec 27, 2019

@ktalebian can you provide a code sample ?

@jansupol
Copy link
Contributor

The way how @Priority works seems to be changed in 2.26, since then Priority.USER is the default and Priority.USER+1 is what can help here with your ExceptionMapper.

This looks like several different issues reported by multiple users, the symptoms are similar, but the cause seems to be different for each report.

@bric3 Can you file your own bug with the reproducer using spring?

@ktalebian Can you file a new bug with your own reproducer?

@cen1
Copy link
Contributor

cen1 commented Nov 5, 2020

I can at least confirm that registering a provider with ExceptionMapper<ConstraintViolationException> works while ExceptionMapper<ValidationException> does not. Jersey 2.28. In most cases you probably want to catch ConstraintViolationException anyway but it would be nice if jersey respected user registered provider and the @Priority chain in case of ValidationException mapper.

@bric3
Copy link

bric3 commented Nov 5, 2020

Sorry for the late reply, I've abandonned JEE webstacks. So I'm not sure how to reproduce this.

@jansupol
Copy link
Contributor

jansupol commented Nov 6, 2020

ExceptionMapper<ValidationException> definitely works. We have a TCK test that uses ExceptionMapper<ValidationException>.

@cen1
Copy link
Contributor

cen1 commented Nov 6, 2020

That's weird. I will try to dive into debugger to figure this one out on my side.

@cen1
Copy link
Contributor

cen1 commented Nov 6, 2020

From what I can understand from stepping through debugger in my project:

  1. Mapper is selected here:
    if (isPreferredCandidate(exceptionInstance, candidate, d == minDistance)) {

    It first finds the jersey provided ValidationExceptionMapper and finds distance d=1, sets minDistance=1.
  2. Then it finds my mapper, but since distance is the same, isPreferredCandidate returns false here

It seems to me that whoever is listed first in the provider list will be selected unless there is deeper logic somewhere else. The TCK test might be a fluke. I can research further with some additional pointers.

I am testing on 2.28 since that is what we use in the framework but it doesn't seem like the code changed in develop.

@jansupol
Copy link
Contributor

jansupol commented Nov 6, 2020

The exceptionMappers are sorted by the priority first here. So if yours is better (at least Priorities.USER or lower) yours should get first in the list.

@jansupol
Copy link
Contributor

jansupol commented Nov 6, 2020

How do you register the exception mapper?

@jansupol
Copy link
Contributor

jansupol commented Nov 6, 2020

Here is a test example that works if the ExceptionMapper is registered into ResourceConfig:

public class BVMapperTest extends JerseyTest {

    @Path("/x")
    public static class ExceptionResource {
        @GET
        public String get(@Positive @QueryParam("i") int i){
            return String.valueOf(i);
        }
    }

    @Priority(Priorities.USER)
    public static class BVExceptionMapper implements ExceptionMapper<ValidationException> {

        @Override
        public Response toResponse(ValidationException exception) {
            return Response.ok("HELLO").build();
        }
    }

    @Override
    protected Application configure() {
        return new ResourceConfig(ExceptionResource.class, BVExceptionMapper.class, ValidationExceptionMapper.class);
    }

    @Test
    public void test() {
        try (Response r = target("/x").queryParam("i", "-2").request().get()) {
            System.out.println(r.getStatus());
            System.out.println(r.readEntity(String.class));
        }
    }
}

Try to play with the priority to see the difference.

@jansupol
Copy link
Contributor

jansupol commented Nov 6, 2020

For registering it to HK2, similar to the following should work:

                .register( new AbstractBinder() {
                    @Override
                    protected void configure() {
                        bind(BVExceptionMapper.class).qualifiedBy(CustomAnnotationLiteral.INSTANCE)
                                .to(ExceptionMapper.class).in(Singleton.class);
                    }
                });

@cen1
Copy link
Contributor

cen1 commented Nov 6, 2020

I register it via Application public Set<Class<?>> getClasses() override.

I came down to this part of the code in Providers.java:

List<ServiceHolder<T>> providers = getServiceHolders(injectionManager, contract, Comparator.comparingInt(Providers::getPriority), CustomAnnotationLiteral.INSTANCE);

This first call only returns two mappers, JsonMappingExceptionMapper and JsonParseExceptionMapper.
The second call is unsorted and returns 84 additional exception mappers, including the jersey's ValidationExceptionMapper and my own. This call is unsorted and @priority is ignored (I have set priority 0).

providers.addAll(getServiceHolders(injectionManager, contract));

So it boils down to CustomAnnotationLiteral.INSTANCE. What makes an ExceptionMapper part of the CustomAnnotationLiteral.INSTANCE? I suspect at this point I am missing some dependencies or something is missing in our appserver jersey integration and it manifests this way. Something to do with HK?

I confirm that explicitely registering to HK2 works (only as instance, not as class).

@jansupol
Copy link
Contributor

jansupol commented Nov 9, 2020

The classes are registered to HK2 in ResourceModelConfigurator class:

ProviderBinder.bindProvider(component, model, injectionManager);

Which in turn calls:

injectionManager.register(createClassBinder(model));

and it makes sure to use CustomAnnotationsLiteral.INSTANCE:

Can you sum up a simple reproducer for your case?

@cen1
Copy link
Contributor

cen1 commented Nov 9, 2020

It has something to do with CDI enabelment. I summed up the observations in the reproducer: https://github.com/cen1/jersey-validation-mapper-reproducer

Guessing strictly from the jersey logs with respect to both Json* mappers in my custom mapper, mappers that are bound by Jersey CDI component provider are the issue.

@jansupol
Copy link
Contributor

I confirm the bug in Jersey 2.28. It has been fixed in 2.29.

@cen1
Copy link
Contributor

cen1 commented Nov 12, 2020

Will update to 2.29 and re-test. I appreciate the support.

@cen1
Copy link
Contributor

cen1 commented Nov 12, 2020

Confirming, upgrading jersey solved the problem for me.

@jansupol
Copy link
Contributor

Cool, closing the issue. Thanks.

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

No branches or pull requests

7 participants