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

with move to feign-core 10.x.x, support capturing the request in Exception constructors #18

Open
saintf opened this issue Mar 4, 2019 · 6 comments

Comments

@saintf
Copy link
Collaborator

saintf commented Mar 4, 2019

Feign core 10.X.X allows capturing the Request made from a Response - meaning that we can include the original request into an exception if required.

The requirement here is to detect if one of the parameters in the constructor for the exception is of type Request, and if so, put the original request into that field.

In a later iteration we can ask people want us to do more parsing (like only include request headers based on a @RequestHeaders tag or only the request body based on a @RequestBody tag) - though for now we expect handing over the Request itself is probably more than enough (would want to see usecases for anything further).

Hence - put simply - when an exception is defined as:

public class MyException {
   
   public MyException(Request request) {
      ...
   }
}

then the AnnotationErrorDecoder should inject the Request.

@StefanFellinger
Copy link

Hi saintf, i think we have time to implement that future request, and create a pull request for you. Do you aggree with it, or fo you prefer to implement it by yourself?

@saintf
Copy link
Collaborator Author

saintf commented Jun 8, 2019

@StefanFellinger Absolutely happy for you to give it a go :)

@StefanFellinger
Copy link

hey @saintf,

i've added logic to the ExceptionGenerator and updated your tests. How can i publish my changes as an pull request or something else. Can you please authorize me to do it.

@saintf
Copy link
Collaborator Author

saintf commented Jun 13, 2019

hi @StefanFellinger - take a look at:

Basically - fork the codebase, make the changes, make sure code style matches/etc, everything is tested. once you're good, raise a PR from your fork, I'll review it, and then when we're done (review complete/etc) I'll merge it and release it :)

@saintf
Copy link
Collaborator Author

saintf commented Jul 22, 2019

Merged #23

@saintf
Copy link
Collaborator Author

saintf commented Jul 22, 2019

@StefanFellinger - Merged and released. It's in maven central (though it will take it about 30 minutes to actually show up in searches since it takes a while for the indexes to be updated/etc).

Make sure it's good to go and feel free to close this issue! Also - if it makes sense, also close #22?

Thanks for the help!

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

2 participants