Avoid reporting Broken Pipe errors to Sentry in a Spring Boot application

Question

I have a Spring Boot application that uses Sentry for exception tracking and I'm getting some errors that look like this:

ClientAbortExceptionorg.apache.catalina.connector.OutputBuffer in realWriteBytes
errorjava.io.IOException: Broken pipe

My understanding is that it's just a networking error and thus I should generally ignore them. What I want to do is report all other IOExceptions and log broken pipes to Librato so I can keep an eye on how many I'm getting (a spike might mean there's an issue with the client, which is also developed by me in Java):

I came up with this:

@ControllerAdvice
@Priority(1)
@Order(1)
public class RestExceptionHandler {
    @ExceptionHandler(ClientAbortException.class)
    @ResponseStatus(HttpStatus.SERVICE_UNAVAILABLE)
    public ResponseEntity<?> handleClientAbortException(ClientAbortException ex, HttpServletRequest request) {
        Throwable rootCause = ex;
        while (ex.getCause() != null) {
            rootCause = ex.getCause();
        }
        if (rootCause.getMessage().contains("Broken pipe")) {
            logger.info("count#broken_pipe=1");
        } else {
            Sentry.getStoredClient().sendException(ex);
        }
        return null;
    }
}

Is that an acceptable way to deal with this problem?

I have Sentry configured following the documentation this way:

@Configuration
public class FactoryBeanAppConfig {
    @Bean
    public HandlerExceptionResolver sentryExceptionResolver() {
        return new SentryExceptionResolver();
    }

    @Bean
    public ServletContextInitializer sentryServletContextInitializer() {
        return new SentryServletContextInitializer();
    }
}

I don't see an issue with the code sample provided, but how is Sentry configured in the rest of the application? If something somewhere else is capturing exceptions and sending them to Sentry then this wouldn't prevent that from happening. Or are you using a Sentry logging integration and only sending things that ERROR or above? — Brett, Feb 21 '18 at 21:25
@Brett: thank you for the comment. I added that information to the question. — Pablo Fernandez, Feb 22 '18 at 07:17
I think that by returning `null` you're telling Spring to run the other handlers, which seems like it would run the Sentry resolver which would send it to Sentry anyway? You may need to return a valid `ResponseEntity` instead? At least that's what I found when implementing the Sentry resolver: https://github.com/getsentry/sentry-java/blob/f57a8e7020527ff077cff4400faabae631560289/sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java#L26-L27 — Brett, Feb 22 '18 at 12:29
@Pablo, I assume this is not working and still sending the request to Sentry? — Tarun Lalwani, Feb 24 '18 at 13:24
@Pablo, you can create a test endpoint and inside that raise a exception `throw new ClientAbortException("exception", new IOException("Broken pipe"))` — Tarun Lalwani, Feb 25 '18 at 12:19
@Pablo could you please elaborate a bit more on how your infrastructure looks like. Broken Pipe usually means that client terminated a connection (e.g. by sending RST packet instead of graceful close with FIN). I had funny case (±40k broken pipes per day) with HaProxy and Netty based backend. Reason - HaProxy configured to use HTTP1.1 with HTTP KeepAlive for heartbeat checks (it closed connection with RST), switch to Http 1.0 solved 99% of them, rest was handled by closing Idle connections from backend side before balancer did it. — Grigory, Feb 27 '18 at 13:28
Hi, I wonder whether we should ignore "broken pipe" error or should report it? Because by default, Sentry and also Elastic APM both report this as error. Personally I think we should ignore it, but why do the two libraries report it? — ch271828n, Mar 18 '21 at 12:09

score 10 · Accepted Answer · edited Jun 22 '18 at 17:19

If you look at the class SentryExceptionResolver

public class SentryExceptionResolver implements HandlerExceptionResolver, Ordered {
    @Override
    public ModelAndView resolveException(HttpServletRequest request,
                                         HttpServletResponse response,
                                         Object handler,
                                         Exception ex) {

        Sentry.capture(ex);

        // null = run other HandlerExceptionResolvers to actually handle the exception
        return null;
    }

    @Override
    public int getOrder() {
        // ensure this resolver runs first so that all exceptions are reported
        return Integer.MIN_VALUE;
    }
}

By returning Integer.MIN_VALUE in getOrder it makes sure that it gets called first. Even though you have set the Priority to 1, it won't work. So you want to change your

@Configuration
public class FactoryBeanAppConfig {
    @Bean
    public HandlerExceptionResolver sentryExceptionResolver() {
        return new SentryExceptionResolver();
    }

    @Bean
    public ServletContextInitializer sentryServletContextInitializer() {
        return new SentryServletContextInitializer();
    }
}

to

@Configuration
public class FactoryBeanAppConfig {
    @Bean
    public HandlerExceptionResolver sentryExceptionResolver() {
        return new SentryExceptionResolver() {
                @Override
                public int getOrder() {
                    // ensure we can get some resolver earlier than this
                    return 10;
                }
         };
    }

    @Bean
    public ServletContextInitializer sentryServletContextInitializer() {
        return new SentryServletContextInitializer();
    }
}

This will ensure you can have your handler can be run earlier. In your code the loop to get rootCause is incorrect

while (ex.getCause() != null) {
    rootCause = ex.getCause();
}

This is a infinite loop as you have used ex instead of rootCause. Even if you correct it, it can still become an infinite loop. When the exception cause returns itself it will be stuck. I have not thoroughly tested it but I believe it should be like below

while (rootCause.getCause() != null && rootCause.getCause() != rootCause) {
    rootCause = rootCause.getCause();
}

The is one way of solving your problem. But you need to send the exception yourself to Sentry. So there is another way to handle your requirement

Way 2

In this case you can do the whole logic in your Configuration and change it to below

@Configuration
public class FactoryBeanAppConfig {
    @Bean
    public HandlerExceptionResolver sentryExceptionResolver() {
        return new SentryExceptionResolver() {
            @Override
            public ModelAndView resolveException(HttpServletRequest request,
                    HttpServletResponse response,
                    Object handler,
                    Exception ex) {
                Throwable rootCause = ex;

                while (rootCause .getCause() != null && rootCause.getCause() != rootCause) {
                    rootCause = rootCause.getCause();
                }

                if (!rootCause.getMessage().contains("Broken pipe")) {
                    super.resolveException(request, response, handler, ex);
                }
                return null;
            }   

        };
    }

    @Bean
    public ServletContextInitializer sentryServletContextInitializer() {
        return new SentryServletContextInitializer();
    }
}

I was reviewing this code and for getting to the bottom exception, shouldn't it be doing `rootCause = rootCause.getCause();`. Otherwise, it can only go one step as `ex` never changes. — Pablo Fernandez, Jun 22 '18 at 17:00
@pupeno, yes you are right. When I tested it only had one level deep exception, so didn't catch this. It should be indeed `rootCause = rootCause.getCause();` — Tarun Lalwani, Jun 22 '18 at 17:05
Cool! I corrected the answer. Surprisingly, I copied the code and just by luck I only ever had to deal with two levels (until now), so, I was very confused. — Pablo Fernandez, Jun 22 '18 at 17:20
Thanks for the update. Good that the error occurred and you could notice the issue :-) — Tarun Lalwani, Jun 22 '18 at 18:45
Hi, I wonder whether we should ignore "broken pipe" error or should report it? Because by default, Sentry and also Elastic APM both report this as error. Personally I think we should ignore it, but why do the two libraries report it? — ch271828n, Mar 18 '21 at 12:09

Avoid reporting Broken Pipe errors to Sentry in a Spring Boot application

1 Answers1