Skip to content

Fixing external timeout issues with 531 instead 500 for better availability #226

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.

Sign up for GitHub

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

Merged
Sathish1891 merged 1 commit into from
Dec 17, 2024
Merged

Fixing external timeout issues with 531 instead 500 for better availability #226

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions http/src/main/java/com/mx/path/connect/http/HttpClientFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.EOFException;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.SocketTimeoutException;
import java.net.URISyntaxException;
import java.time.LocalDate;
import java.time.LocalDateTime;
Expand Down Expand Up @@ -35,13 +36,15 @@

import org.apache.http.HttpEntity;
import org.apache.http.NameValuePair;
import org.apache.http.NoHttpResponseException;
import org.apache.http.client.HttpClient;
import org.apache.http.client.ResponseHandler;
import org.apache.http.client.config.CookieSpecs;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.entity.UrlEncodedFormEntity;
import org.apache.http.client.methods.RequestBuilder;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.conn.ConnectTimeoutException;
import org.apache.http.conn.HttpHostConnectException;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.entity.ByteArrayEntity;
Expand All @@ -65,6 +68,7 @@ public class HttpClientFilter extends RequestFilterBase {
* instead of converting it to a String.
*/
private static final List<String> RAW_BODY_CONTENT_TYPE_HINTS = Arrays.asList("image", "pdf", "msword");
private static final int HTTP_STATUS_EXTERNAL_TIMEOUT = 531;

private static GsonBuilder gsonBuilder = new GsonBuilder();
private static final Gson GSON = gsonBuilder
Expand Down Expand Up @@ -157,6 +161,15 @@ public final void execute(Request request, Response response) {
} finally {
response.finish();
}
} catch (ConnectTimeoutException e) {
httpResponse.setStatus(HttpStatus.valueOf(HTTP_STATUS_EXTERNAL_TIMEOUT));
Copy link
Collaborator

@mattnichols mattnichols Jan 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the status here is meaningless if you are just going to immediately raise an exception.

Copy link
Collaborator

@mattnichols mattnichols Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may be trying to solve this too close to the call. Have you looked at the connection filters? That may be a more appropriate place to solve for this.

It looks like these are exceptions that are raised by HTTPClient, so, this is the right place to handle them. We should raise them as HttpClientConnectExceptions. There is no reason for the extra constant. We should use the the HttpStatus enum.

throw new ConnectException("Connection timeout: " + e.getMessage(), e);
} catch (SocketTimeoutException e) {
httpResponse.setStatus(HttpStatus.valueOf(HTTP_STATUS_EXTERNAL_TIMEOUT));
throw new ConnectException("Read timeout: " + e.getMessage(), e);
} catch (NoHttpResponseException e) {
httpResponse.setStatus(HttpStatus.valueOf(HTTP_STATUS_EXTERNAL_TIMEOUT));
throw new ConnectException("Target server failed to respond: " + e.getMessage(), e);
} catch (SSLHandshakeException e) {
throw new HttpClientConnectException("SSL handshake failed", e);
} catch (SSLException e) {
Expand Down
Loading