Skip to content

Commit 319e98d

Browse files
mcollovatimcollovati
authored
feat: prevent logging of ignored exceptions (#17069)
* feat: prevent logging of ignored exceptions Flow default error handler logs all caught exceptions. However, there may be exceptions thrown by servlet container that are meant to be tracked only at debug level (e.g. Jetty QuietException implementors). This change prevents logging of similar 'quiet' exception, unless the DefaultErrorHandler logger level is set to debug or higher. Closes #16311 * add exceptions to ignore * update javadoc
1 parent 75e3ed1 commit 319e98d

File tree

2 files changed

+181
-8
lines changed

2 files changed

+181
-8
lines changed

‎flow-server/src/main/java/com/vaadin/flow/server/DefaultErrorHandler.java

+52-8
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616

1717
package com.vaadin.flow.server;
1818

19+
import java.io.EOFException;
20+
import java.net.SocketException;
21+
import java.net.SocketTimeoutException;
22+
import java.util.Set;
23+
1924
import org.slf4j.Logger;
2025
import org.slf4j.LoggerFactory;
2126
import org.slf4j.Marker;
@@ -26,25 +31,64 @@
2631
/**
2732
* The default implementation of {@link ErrorHandler}.
2833
*
34+
* This implementation logs the exception at ERROR level, unless the exception
35+
* is in the ignore list.
36+
*
37+
* By default, the following exceptions are ignored to prevent logs to be
38+
* flooded by errors that are usually not raised by application logic, but are
39+
* caused by external event, such as broken connections or network issues.
40+
*
41+
* <ul>
42+
* <li>java.net.SocketException</li>
43+
* <li>java.net.SocketTimeoutException</li>
44+
* <li>java.io.EOFException</li>
45+
* <li>org.apache.catalina.connector.ClientAbortException</li>
46+
* <li>org.eclipse.jetty.io.EofException</li>
47+
* </ul>
48+
*
49+
* If the handler logger is set to DEBUG level, all exceptions are logged,
50+
* despite they are in the ignore list.
51+
*
2952
* @author Vaadin Ltd
3053
* @since 1.0
3154
*/
3255
public class DefaultErrorHandler implements ErrorHandler {
56+
57+
private final Set<String> ignoredExceptions;
58+
59+
protected DefaultErrorHandler(Set<String> ignoredExceptions) {
60+
this.ignoredExceptions = Set.copyOf(ignoredExceptions);
61+
}
62+
63+
public DefaultErrorHandler() {
64+
this.ignoredExceptions = Set.of(SocketException.class.getName(),
65+
SocketTimeoutException.class.getName(),
66+
EOFException.class.getName(),
67+
"org.eclipse.jetty.io.EofException",
68+
"org.apache.catalina.connector.ClientAbortException");
69+
}
70+
3371
@Override
3472
public void error(ErrorEvent event) {
3573
Throwable throwable = findRelevantThrowable(event.getThrowable());
36-
37-
Marker marker = MarkerFactory.getMarker("INVALID_LOCATION");
38-
if (throwable instanceof InvalidLocationException) {
39-
if (getLogger().isWarnEnabled(marker)) {
40-
getLogger().warn(marker, "", throwable);
74+
if (shouldHandle(throwable)) {
75+
Marker marker = MarkerFactory.getMarker("INVALID_LOCATION");
76+
if (throwable instanceof InvalidLocationException) {
77+
if (getLogger().isWarnEnabled(marker)) {
78+
getLogger().warn(marker, "", throwable);
79+
}
80+
} else {
81+
// print the error on console
82+
getLogger().error("", throwable);
4183
}
42-
} else {
43-
// print the error on console
44-
getLogger().error("", throwable);
4584
}
4685
}
4786

87+
protected boolean shouldHandle(Throwable t) {
88+
return getLogger().isDebugEnabled()
89+
|| !ignoredExceptions.contains(t.getClass().getName());
90+
}
91+
4892
/**
4993
* Vaadin wraps exceptions in its own and due to reflection usage there
5094
* might be also other irrelevant exceptions that make no sense for Vaadin

‎flow-server/src/test/java/com/vaadin/flow/server/DefaultErrorHandlerTest.java

+129
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* Copyright 2000-2023 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
17+
package com.vaadin.flow.server;
18+
19+
import java.io.IOException;
20+
import java.io.UncheckedIOException;
21+
import java.net.MalformedURLException;
22+
import java.util.Set;
23+
24+
import org.junit.After;
25+
import org.junit.Before;
26+
import org.junit.Test;
27+
import org.mockito.ArgumentMatchers;
28+
import org.mockito.MockedStatic;
29+
import org.mockito.Mockito;
30+
import org.slf4j.Logger;
31+
import org.slf4j.LoggerFactory;
32+
33+
public class DefaultErrorHandlerTest {
34+
35+
MockedStatic<LoggerFactory> loggerFactory;
36+
Logger logger;
37+
38+
@Before
39+
public void setUp() throws Exception {
40+
logger = Mockito
41+
.spy(LoggerFactory.getLogger(DefaultErrorHandler.class));
42+
loggerFactory = Mockito.mockStatic(LoggerFactory.class);
43+
loggerFactory
44+
.when(() -> LoggerFactory
45+
.getLogger(DefaultErrorHandler.class.getName()))
46+
.thenReturn(logger);
47+
Mockito.when(logger.isDebugEnabled()).thenReturn(false);
48+
}
49+
50+
@After
51+
public void tearDown() throws Exception {
52+
loggerFactory.close();
53+
}
54+
55+
@Test
56+
public void error_acceptedException_errorHandled() {
57+
DefaultErrorHandler errorHandler = Mockito
58+
.spy(new DefaultErrorHandler(Set.of(IOException.class.getName(),
59+
MalformedURLException.class.getName())));
60+
61+
Throwable throwable = new RuntimeException();
62+
errorHandler.error(new ErrorEvent(throwable));
63+
Mockito.verify(logger).error("", throwable);
64+
65+
throwable = new IllegalArgumentException();
66+
errorHandler.error(new ErrorEvent(throwable));
67+
Mockito.verify(logger).error("", throwable);
68+
}
69+
70+
@Test
71+
public void error_ignoredException_notHandled() {
72+
DefaultErrorHandler errorHandler = Mockito
73+
.spy(new DefaultErrorHandler(Set.of(IOException.class.getName(),
74+
MalformedURLException.class.getName(),
75+
"com.vaadin.flow.server.DefaultErrorHandlerTest$InnerException")));
76+
77+
errorHandler.error(new ErrorEvent(new IOException()));
78+
errorHandler.error(new ErrorEvent(new MalformedURLException()));
79+
errorHandler.error(new ErrorEvent(new InnerException()));
80+
81+
Mockito.verify(logger, Mockito.never()).error(
82+
ArgumentMatchers.anyString(),
83+
ArgumentMatchers.any(Throwable.class));
84+
}
85+
86+
@Test
87+
public void error_subclassOfIgnoredException_errorHandled() {
88+
DefaultErrorHandler errorHandler = Mockito.spy(
89+
new DefaultErrorHandler(Set.of(IOException.class.getName())));
90+
91+
Throwable throwable = new MalformedURLException();
92+
errorHandler.error(new ErrorEvent(throwable));
93+
Mockito.verify(logger).error("", throwable);
94+
}
95+
96+
@Test
97+
public void error_loggerAtDebugLevel_errorHandled() {
98+
Mockito.reset(logger);
99+
Mockito.doReturn(true).when(logger).isDebugEnabled();
100+
101+
DefaultErrorHandler errorHandler = Mockito
102+
.spy(new DefaultErrorHandler(Set.of(IOException.class.getName(),
103+
MalformedURLException.class.getName(),
104+
"com.vaadin.flow.server.DefaultErrorHandlerTest$InnerException")));
105+
106+
Throwable throwable = new RuntimeException();
107+
errorHandler.error(new ErrorEvent(throwable));
108+
Mockito.verify(logger).error("", throwable);
109+
110+
throwable = new IOException();
111+
errorHandler.error(new ErrorEvent(throwable));
112+
Mockito.verify(logger).error("", throwable);
113+
114+
throwable = new MalformedURLException();
115+
errorHandler.error(new ErrorEvent(throwable));
116+
Mockito.verify(logger).error("", throwable);
117+
118+
throwable = new InnerException();
119+
errorHandler.error(new ErrorEvent(throwable));
120+
Mockito.verify(logger).error("", throwable);
121+
122+
throwable = new UncheckedIOException(new IOException());
123+
errorHandler.error(new ErrorEvent(throwable));
124+
Mockito.verify(logger).error("", throwable);
125+
}
126+
127+
public static class InnerException extends Exception {
128+
}
129+
}

0 commit comments

Comments
 (0)