OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.46k stars 1.92k forks source link

Feign Soap Exceptions #1479

Open alabotski opened 3 years ago

alabotski commented 3 years ago

JAXB by default silently ignores errors.
Can you add this code to throw an exception if something goes wrong. To SoapEncoder & SoapDecoder


 unmarshaller.setEventHandler(
    new ValidationEventHandler() {
        @Override
        public boolean handleEvent(ValidationEvent event ) {
            throw new RuntimeException(event.getMessage(),
                                       event.getLinkedException());
        }
});
virtual-machinist commented 2 years ago

By javax.xml.bind.ValidationEventHandler#handleEvent contract you can simply return false and JAXB will throw an exception:

unmarshaller.setEventHandler(event -> false);

However I wouldn't add this sort of handling by default, as all of the written code already relies on the default JAXB behavior and instead provide means to customize feign.jaxb.JAXBContextFactory.Builder.

kdavisk6 commented 2 years ago

I agree with @virtual-machinist, this is something you'll have to provide to the SOAP encoder/decoder components. Those components should accept an external JAXBContext. If not, please open an enhancement issue/pr to do so.

virtual-machinist commented 1 year ago

@kdavisk6 I'm afraid it's not that simple and we have to modify feign.jaxb.JAXBContextFactory even if we allow external JAXBContext for SOAP encoders and decoders.

AFAIK you can only set the ValidationEventHandler when Marshaller and Unmarshaller instances are created, i.e. at some point in feign.jaxb.JAXBContextFactory#createUnmarshaller or feign.jaxb.JAXBContextFactory#createMarshaller.

In other words something like

public Builder withUnmarshallerValidationEventHandler(ValidationEventHandler handler) {
 ...
}

public Builder withMarshallerValidationEventHandler(ValidationEventHandler handler) {
 ...
}

is needed + modifications to the create factory methods.

If this is a reasonable proposal, I can make a branch + PR. :smiley:

virtual-machinist commented 1 year ago

@kdavisk6 I've created a PR (#2084) that hopefully addresses this issue.