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

Intercepted values are preserved from previous request on retry #2376

Open
derklaro opened this issue Apr 6, 2024 · 2 comments
Open

Intercepted values are preserved from previous request on retry #2376

derklaro opened this issue Apr 6, 2024 · 2 comments
Labels
documentation Issues that require updates to our documentation feedback provided Feedback has been provided to the author

Comments

@derklaro
Copy link
Contributor

derklaro commented Apr 6, 2024

I'm using feign to call a backend API that needs a checksum of the request body as a query parameter. For this purpose I've implemented a RequestInterceptor which applies that query parameter to the request. Due to some issues with that API (f. ex. it being down for a few seconds) I also have a retrier applied, that retries the request in case it fails.

I just noticed an issue with that setup: the checkstum query parameter set in the previous request (that failed and should get retried) is not removed and the request is applied to the same request interceptor again. Due to the issue that there is no way to set a query parameter (only to append) which isn't clear from the method naming (query & appendQuery while both append to the query) the checksum is applied multiple times, causing the request to fail again as the backend (for some reason, don't ask me why) cannot handle multiple checksum parameters provided in the request:
<backend url>?checksum=1da6e6d858608e64d5e4a9798c589833&checksum=1da6e6d858608e64d5e4a9798c589833&checksum=1da6e6d858608e64d5e4a9798c589833&checksum=1da6e6d858608e64d5e4a9798c589833&checksum=1da6e6d858608e64d5e4a9798c589833&checksum=1da6e6d858608e64d5e4a9798c589833

For now I will just go ahead and remove the previous query parameter (calling query("checksum", List.of())), but it would be nice if the old request without the intercepted values could be passed to the retrier to prevent these issues (not calling the interceptors again might be another solution, however, maybe there are apis that need f. ex. a time-based checksum that requires re-computing the checksum). Another solution might be to just add setQuery/setHeader methods to override the previous query/header value explicitly.

I didn't validate yet, but I am pretty sure that issue also applies to headers.

@kdavisk6
Copy link
Member

Calling query as you've specified is the correct usage to clear a query value.

 public RequestTemplate query(String name, String... values) {
    if (values == null) {
      return query(name, Collections.emptyList());
    }
    return query(name, Arrays.asList(values));
  }

When it comes to retrying requests, Feign retries the entire request building chain to support situations you've described, like updating headers or query strings with time-sensitive information or nonce style values. In short, Feign makes no assumptions on how a header or query string should be handled, either single or multiple value, and delegate that responsibility to the caller.

We recommend that if you want to use our retry capability and have request specific headers ore query strings, that you explicitly clear the values using an interceptor.

@kdavisk6 kdavisk6 added documentation Issues that require updates to our documentation feedback provided Feedback has been provided to the author labels Sep 11, 2024
@derklaro
Copy link
Contributor Author

Yea, I'm clearing the headers and/or query parameters in each interceptor now. However, it would be nice to have a method which explicitly sets the value and removes all previously set values as clearing each time is a bit tideous like:

template.query("q1", null).query("q1", "va1l").query("q2", null).query("q2", "val2")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues that require updates to our documentation feedback provided Feedback has been provided to the author
Projects
None yet
Development

No branches or pull requests

2 participants