OpenFeign / feign-annotation-error-decoder

Apache License 2.0
51 stars 9 forks source link

Use own ExceptionGenerator #22

Closed MariusKl closed 5 years ago

MariusKl commented 5 years ago

Hi,

I would like to use my own ExceptionGenerator to be free to add another field (requestUrl) to my exception. Could you adapt the AnnotationErrorDecoder to be able to pass an ExceptionGeneratorBuilder to this class?

I thought about adding a new method

public Builder withExceptionGeneratorBuilder(ExceptionGenerator.Builder exceptionGenerator) { this.exceptionGenerator = exceptionGenerator; return this; }

to the AnnotationErrorDecoder, but due to the current design this would lead to a lot of changes in the ExceptionGenerator and ExceptionGeneratorBuilder classes and its design.

saintf commented 5 years ago

Rather than doing that (Which is basically taking a ton of the heart away) would it be better if instead we allowed you to capture the request object provided by Feign?

You could hence perhaps take the request as a parameter and then use its internals to capture requestUrl/headers/etc - thoughts?

StefanFellinger commented 5 years ago

It's an idea. Would be fine for us. Mabe you can provide a single DTO Object that holds request, response and the body or the method that resolves it. Then Exception Constructors need less parameters and the DTO can be extended when things change...

saintf commented 5 years ago

Take a look at https://github.com/OpenFeign/feign-annotation-error-decoder/issues/18 - the idea is that it would then return a standard Feign Request object (look at https://github.com/OpenFeign/feign/blob/master/core/src/main/java/feign/Request.java) which you can then use to get resolved URI/Headers/etc if you want.

If that looks good, let's close this ticket out and upvote that one. Are you ok to give it a try at making the changes and I can review afterwards?

MariusKl commented 5 years ago

Thank you for adding this in #18