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

Support Response Interceptors #1126

Open
DogSunny opened this issue Nov 30, 2019 · 12 comments
Open

Support Response Interceptors #1126

DogSunny opened this issue Nov 30, 2019 · 12 comments
Labels
proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration

Comments

@DogSunny
Copy link

In my project, I use the request interceptor to sign the request. I thought that there would be a corresponding response interceptor to verify the signature, but I didn't find it. Is there a better way?

@velo
Copy link
Member

velo commented Dec 4, 2019

Short answer is cause nobody implemented it.

As workaround, you could use a customized decoded to validate your response and then delegate decoding to whatever decoder you need.

@kdavisk6 kdavisk6 added the feedback provided Feedback has been provided to the author label Jan 14, 2020
@mox601
Copy link

mox601 commented Apr 7, 2020

I also could use a response interceptor: my use case is to capture some response headers and run some logic on them, possibly only if the response is successful (e.g. only when the status is 200 and the decoding throws no exceptions).

What would be the appropriate place to do such change, design-wise?
Maybe in SynchronousMethodHandler.executeAndDecode(...), right after the decoder runs the method decoder.decode(response, metadata.returnType());?
(I am not considering the asyncResponseHandler because it's marked as @Experimental)

@velo
Copy link
Member

velo commented Apr 7, 2020

@Experimental is just for the sake of informing API may change in a unpredictable way and break compatibility between minor releases.

Just to give us a little extra flexibility =)

So, RequestInterceptor happens right after encoder and before client is invoked...
ResponseInterceptor then would happen after client, before decoder. So yes, SynchronousMethodHandler.executeAndDecode seems to be the right place, just not after decoder, but before.

Now, what are you trying to do that you can't do by chain decoders?

class MyResponseValidatorDecoder implements Decoder {
  private final Decoder realDecoder; //populated by constructor

  // inside decode method
{
  if(response.getStatus() == 302) return new RedirectObject(response);
  return realDecoder.decode();
}

}

//usage
FeignBuilder.decoder(new MyResponseValidatorDecoder(new JacksonDecoder()));

@qrqhuang
Copy link

How can i send another request when response in some status, and then retry request.

The steps like this.

request: biz -> reponse: access_token expire -> request: refresh access_token ->request: retry biz

@kdavisk6 kdavisk6 changed the title Why is there no response interceptor? Support Response Interceptors Dec 29, 2020
@kdavisk6 kdavisk6 added proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration and removed feedback provided Feedback has been provided to the author labels Dec 29, 2020
@kdavisk6
Copy link
Member

I've updated the title of this issue to better reflect the ask and added the proposal label. If this receives enough support we will consider this enhancement.

@kpramesh2212
Copy link

This would be a great feature if we can get it.

@naushadmohammed
Copy link

I too have a use case where I can leverage ResponseInterceptor functionality. Will be good to see this implemented, instead of performing a workaround.

@Bloof
Copy link

Bloof commented Dec 7, 2021

I too have a need for this kind of functionality even though my work around is working as well. The response interceptor just would need to happen before decoder. In this case all data is moved around in encrypted string so it is needed to be decrypted before say GsonDecoder can access and decode the json.

My current decryption decoder is like so if someone finds it useful:

`public class GsonDecryptionDecoder extends GsonDecoder {

@Override
public Object decode(Response response, Type type) throws IOException, FeignException {

    try (Reader reader = response.body().asReader(Charset.defaultCharset())) {

        String encryptedMessage = CharStreams.toString(reader);
        String decryptedBody = decryptJsonResp(encryptedMessage);

        Response decryptedResponse = Response.builder()
                .body(decryptedBody, Charset.defaultCharset())
                .headers(response.headers())
                .reason(response.reason())
                .status(response.status())
                .request(response.request())
                .build();

        return super.decode(decryptedResponse, type);
    } catch (Exception e) {
        throw new DecodeException(response.status(), "Failed to decode response", response.request(), e);
    }
}

}`

@ondrejpar
Copy link

I also need ResponseInterceptor, especially because decoder workaround is not usable for me - the decoder is not called for methods returning void. I need to validate response headers and ignore the response if it does not contain certain header.

@atesibrahim
Copy link

atesibrahim commented Apr 9, 2022

Hi @kdavisk6 ,
I also need ResponseInterceptor. Could you please consider enhancing it?

I need to catch a header datum in Response to validate it. The following solution is working, but It is ugly. Instead of this I want to write ResponseInterceptor to catch the header rate-limit datum.

At the moment, I appreciate any workaround helps.

Response response = myService.callMethod(request);

// for catching header data
Integer rateLimit = Integer.valueOf(response.headers().get("x-rate-limit-remaining").stream().findFirst().orElse("0")); 

// For getting body
Decoder.Default decoder = new Decoder.Default();
        CustomResponse customResponse;
        try {
            String body = (String) decoder.decode(response, String.class);
            ObjectMapper mapper = new ObjectMapper();
            customResponse = mapper.readValue(body, CustomResponse.class);
        } catch (Exception e) {
            log.error("Error occured during realty transfer response decoding. Error: {}", e.getMessage());
            return;
        }

Huge thnx

@velo
Copy link
Member

velo commented Apr 9, 2022

We are open to evaluating any proposals for this.

Feel free to open a PR and tag me to review.

feiyanke pushed a commit to feiyanke/feign that referenced this issue Apr 28, 2022
velo added a commit that referenced this issue Jun 23, 2022
* add ResponseInterceptor support #1126

* Add the license header.

Add the license header.

Co-authored-by: Dewald de Jager <DewaldDeJager@users.noreply.github.com>

* small fix for license header

* fix format issue

* combine before and after method to one aroundDecode method

* Change ResponseInterceptor to use InvocationContext

Co-authored-by: Fei,Yanke <yanke.fei@mosi-tech.com>
Co-authored-by: feiyanke <feiyanke@126.com>
Co-authored-by: Dewald de Jager <DewaldDeJager@users.noreply.github.com>
@PankajChandaTide
Copy link

Would be good to also be able to throw custom exceptions from decoder or response interceptor, currently all exceptions get wrapped in DecodeException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration
Projects
None yet
Development

No branches or pull requests