-
Notifications
You must be signed in to change notification settings - Fork 204
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
Set up AsyncHttpClient to follow redirects #688
Set up AsyncHttpClient to follow redirects #688
Conversation
By default the AsyncHttpClient does not follow redirects. The `LeaderRedirectionFilter` redirects clients to the current leader as relevant. When the agents reach out to the leader, they are getting a redirect but failing to follow the redirect automatically causing an error like ```java java.lang.Exception: response=NettyResponse { statusCode=302 headers= Access-Control-Allow-Origin: * Location: http://new-leader/api/v1/resourceClusters/log-ingest/actions/heartBeatFromTaskExecutor Server: akka-http/10.2.7 Date: Tue, 09 Jul 2024 15:33:46 GMT Content-Type: text/html; charset=UTF-8 content-length: 202 body= The requested resource temporarily resides under <a href="http://new-leader/api/v1/resourceClusters/log-ingest/actions/heartBeatFromTaskExecutor">this URI</a>. } at io.mantisrx.server.master.resourcecluster.ResourceClusterGatewayClient.lambda$performAction$0(ResourceClusterGatewayClient.java:106) at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1072) at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:506) at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2079) at org.asynchttpclient.netty.NettyResponseFuture.loadContent(NettyResponseFuture.java:222) at org.asynchttpclient.netty.NettyResponseFuture.done(NettyResponseFuture.java:257) at org.asynchttpclient.netty.handler.AsyncHttpClientHandler.finishUpdate(AsyncHttpClientHandler.java:241) at org.asynchttpclient.netty.handler.HttpHandler.handleChunk(HttpHandler.java:113) at org.asynchttpclient.netty.handler.HttpHandler.handleRead(HttpHandler.java:142) at org.asynchttpclient.netty.handler.AsyncHttpClientHandler.channelRead(AsyncHttpClientHandler.java:78) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436) at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:327) at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:299) at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166) at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:722) at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:658) at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:584) at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:496) at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:995) at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) at java.base/java.lang.Thread.run(Thread.java:829) Caused by: io.mantisrx.shaded.com.fasterxml.jackson.core.JsonParseException: Unrecognized token 'The': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false') at [Source: (String)"The requested resource temporarily resides under <a href="http://new-leader-host/api/v1/resourceClusters/log-ingest/actions/heartBeatFromTaskExecutor">this URI</a>."; line: 1, column: 4] at io.mantisrx.shaded.com.fasterxml.jackson.core.JsonParser._constructError(JsonParser.java:2337) at io.mantisrx.shaded.com.fasterxml.jackson.core.base.ParserMinimalBase._reportError(ParserMinimalBase.java:720) at io.mantisrx.shaded.com.fasterxml.jackson.core.json.ReaderBasedJsonParser._reportInvalidToken(ReaderBasedJsonParser.java:2902) at io.mantisrx.shaded.com.fasterxml.jackson.core.json.ReaderBasedJsonParser._handleOddValue(ReaderBasedJsonParser.java:1949) at io.mantisrx.shaded.com.fasterxml.jackson.core.json.ReaderBasedJsonParser.nextToken(ReaderBasedJsonParser.java:781) at io.mantisrx.shaded.com.fasterxml.jackson.databind.ObjectMapper._initForReading(ObjectMapper.java:4684) at io.mantisrx.shaded.com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4586) at io.mantisrx.shaded.com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3548) at io.mantisrx.shaded.com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3516) at io.mantisrx.server.master.resourcecluster.ResourceClusterGatewayClient.lambda$performAction$0(ResourceClusterGatewayClient.java:103) ... 36 more ``` I made `followRedirect` default to `true` due to `LeaderRedirectionFilter` automatically redirecting. It felt like that was the correct behavior in most cases. However, I also made it configurable to keep it consistent with the other AsyncHttpClient non-default configurations.
@@ -128,6 +128,7 @@ private AsyncHttpClient buildCloseableHttpClient(CoreConfiguration configuration | |||
.setConnectTimeout(configuration.getAsyncHttpClientConnectionTimeoutMs()) | |||
.setRequestTimeout(configuration.getAsyncHttpClientRequestTimeoutMs()) | |||
.setReadTimeout(configuration.getAsyncHttpClientReadTimeoutMs()) | |||
.setFollowRedirect(configuration.getAsyncHttpClientFollowRedirect()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious how does redirect work for you in this case? This communication is from control plane to an agent which should not get a new IP in general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. @timmartin-stripe I think we want to catch the location in which communication is coming in the opposite direction. From agent to master is where we run into real issues.
By default the AsyncHttpClient does not follow redirects. The
LeaderRedirectionFilter
redirects clients to the current leader as relevant. When the agents reach out to the leader, they are getting a redirect but failing to follow the redirect automatically causing an error likeI made
followRedirect
default totrue
due toLeaderRedirectionFilter
automatically redirecting. It felt like that was the correct behavior in most cases. However, I also made it configurable to keep it consistent with the other AsyncHttpClient non-default configurations.Context
Explain context and other details for this pull request.
Checklist
./gradlew build
compiles code correctly./gradlew test
passes all tests