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

Whitelabel Connection Timeout should instead be a handled Gateway Timeout #122

Open
kaladay opened this issue Mar 22, 2021 · 1 comment
Open

Comments

@kaladay
Copy link
Collaborator

kaladay commented Mar 22, 2021

When outside the internal network, my connection will (correctly) timeout.
A ConnectException is being thrown, which is not being handled by the existing @ControllerAdvice.

I think CatalogService is acting as a "gateway" to OKAPI/FOLIO/etc.., so this error response feels more appropriate to me than an internal server error.

I do not think the ConnectException should be directly handled in this case because a ConnectException is too broad and we need to specifically handle Gateway Timeouts and report them. (There may be a desire to also handle all ConnectException, so this could be two distinct changes).

I am suggesting that the code that is performing a edge/okapi requests to catch ConnectException and then construct and thrown a Gateway Exception that should already be handled by the existing HttpServerErrorException advice. This would allow us to provide a more specific message, namely "Gateway Timeout", and possibly reference the service, such as:
504: Gateway Timeout while connecting to OKAPI.

The whitelabel for ConnectException in any other situation could still be handling with custom controller advice, but this would not be a Gateway Timeout and could probably be just set to an Internal Exception, such as:

    @ExceptionHandler(ConnectException.class)
    public ResponseEntity<String> parseError(ConnectException e, WebRequest request) {
        logger.error(e.getMessage());

        if (logger.isDebugEnabled()) {
            e.printStackTrace();
        }

        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
            .contentType(MediaType.TEXT_PLAIN)
            .body(e.getMessage());
    }
@kaladay
Copy link
Collaborator Author

kaladay commented Mar 22, 2021

A snippet of the exception looks like:

2021-03-22 09:56:46.251 ERROR 19367 --- [nio-9000-exec-1] e.t.c.exception.RestExceptionHandler     : Connection timed out (Connection timed
java.net.ConnectException: Connection timed out (Connection timed out)
        at java.net.PlainSocketImpl.socketConnect(Native Method)
        at java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
        at java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
        at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
        at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
        at java.net.Socket.connect(Socket.java:607)
        at sun.security.ssl.SSLSocketImpl.connect(SSLSocketImpl.java:284)
        at sun.security.ssl.BaseSSLSocketImpl.connect(BaseSSLSocketImpl.java:173)
        at sun.net.NetworkClient.doConnect(NetworkClient.java:180)
        at sun.net.www.http.HttpClient.openServer(HttpClient.java:463)
        at sun.net.www.http.HttpClient.openServer(HttpClient.java:558)
        at sun.net.www.protocol.https.HttpsClient.<init>(HttpsClient.java:264)
        at sun.net.www.protocol.https.HttpsClient.New(HttpsClient.java:367)
        at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.getNewHttpClient(AbstractDelegateHttpsURLConnection.java:191)
        at sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1162)
        at sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:1056)
        at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:177)
        at sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:167)
        at org.springframework.http.client.SimpleBufferingClientHttpRequest.executeInternal(SimpleBufferingClientHttpRequest.java:78)
        at org.springframework.http.client.AbstractBufferingClientHttpRequest.executeInternal(AbstractBufferingClientHttpRequest.java:48)
        at org.springframework.http.client.AbstractClientHttpRequest.execute(AbstractClientHttpRequest.java:53)
        at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:661)
        at org.springframework.web.client.RestTemplate.execute(RestTemplate.java:622)
        at org.springframework.web.client.RestTemplate.getForObject(RestTemplate.java:296)
        at edu.tamu.catalog.service.FolioCatalogService.getFeesFines(FolioCatalogService.java:137)
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant