Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

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();
    }
}
like image 377
pupeno Avatar asked Feb 21 '18 19:02

pupeno


1 Answers

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();
    }
}
like image 66
Tarun Lalwani Avatar answered Nov 11 '22 09:11

Tarun Lalwani