Closed samizzy closed 1 year ago
👎 lets find other way to achieve the same
@adyach Open to suggestions, do you have something in mind?
Just for context, the interface TokenInfoRequestExecutor
(snippet below) doesnt allow to throw exceptions and every implementation class returns the error with key error
. So there is no possibility to catch IO exceptions as such. The caller of this method getMap
is TokenInfoResourceServerTokenServices#loadAuthentication
which throws all error messages with InvalidTokenException
public interface TokenInfoRequestExecutor {
Map<String, Object> getMap(final String accessToken);
}
from the quick look there are many options:
1) TokenInfoResourceServerTokenServices is a part of the Zalando libraries, it could be a way to improve overall
2) You can always extend TokenInfoResourceServerTokenServices and change the logic (yes, some duplication of the code)
3) There is also extension NakadiTokenInfoRequestExecutor which overrides getMap
, if there is not other option I would go with adding more sounding error description to catch.
...
and to the why I think we should do it in the other way:
IO error
can be due to different reasons, one of them is timeout. It potentially servers for other errors which you are not aware of. This will increase the load on other endpoints.if(!ex.getMessage().contains("I/O error")) {
throw ex;
}
from the quick look there are many options:
- TokenInfoResourceServerTokenServices is a part of the Zalando libraries, it could be a way to improve overall
- You can always extend TokenInfoResourceServerTokenServices and change the logic (yes, some duplication of the code)
- There is also extension NakadiTokenInfoRequestExecutor which overrides
getMap
, if there is not other option I would go with adding more sounding error description to catch. ...
I have gone with suggestion 3 here, although its still catching general IOException.
some minor comments to fix otherwise better than before 👍
👍
:+1:
Description
In Nakadi, there is logic to retry load token info first from local token and then fallback to remote token info on failure. This logic does not account for when the local token info fails with timeout/IO exception. This PR introduces fix so that it accounts for IO exception and retries remote token info.
Review