Skip to content

Commit a77a1a1

Browse files
committed
[CXF-9189]Exceptions Not Logged When Using SseEventSink in JAX-RS SSE… (#2919)
* [CXF-9189]Exceptions Not Logged When Using SseEventSink in JAX-RS SSE Endpoint * [CXF-9189]use in memory log appender for the testcase (cherry picked from commit ee04e4a) (cherry picked from commit b26d080)
1 parent 10657f4 commit a77a1a1

File tree

5 files changed

+147
-0
lines changed

5 files changed

+147
-0
lines changed

rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/JAXRSInvoker.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.concurrent.CancellationException;
3030
import java.util.concurrent.CompletionException;
3131
import java.util.concurrent.CompletionStage;
32+
import java.util.logging.Level;
3233
import java.util.logging.Logger;
3334

3435
import jakarta.ws.rs.WebApplicationException;
@@ -38,6 +39,7 @@
3839
import jakarta.ws.rs.core.MediaType;
3940
import jakarta.ws.rs.core.MultivaluedMap;
4041
import jakarta.ws.rs.core.Response;
42+
import jakarta.ws.rs.sse.SseEventSink;
4143
import org.apache.cxf.common.classloader.ClassLoaderUtils;
4244
import org.apache.cxf.common.classloader.ClassLoaderUtils.ClassLoaderHolder;
4345
import org.apache.cxf.common.i18n.BundleUtils;
@@ -144,6 +146,21 @@ private Object handleAsyncFault(Exchange exchange, AsyncResponseImpl ar, Throwab
144146
try {
145147
return handleFault(new Fault(t), exchange.getInMessage(), null, null);
146148
} catch (Fault ex) {
149+
//If we got here, the fault was effectively "unmapped" (no Response could be created)
150+
// and we'd otherwise lose the usual fault logging (common with SSE sink / async paths).
151+
Throwable cause = ex.getCause() == null ? ex : ex.getCause();
152+
LOG.log(Level.SEVERE, "Unhandled exception from JAX-RS invocation (async/SSE path)", cause);
153+
154+
// Best-effort: if this request is SSE, close the sink so the container can complete cleanly.
155+
try {
156+
SseEventSink sink = (SseEventSink)exchange.getInMessage().get(SseEventSink.class);
157+
if (sink != null && !sink.isClosed()) {
158+
sink.close();
159+
}
160+
} catch (Exception ignore) {
161+
// ignore secondary failures; primary goal is to log original cause
162+
}
163+
147164
ar.setUnmappedThrowable(ex.getCause() == null ? ex : ex.getCause());
148165
if (isSuspended(exchange)) {
149166
ar.reset();

systests/rs-sse/rs-sse-base/pom.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,9 @@
7575
<artifactId>awaitility</artifactId>
7676
<scope>compile</scope>
7777
</dependency>
78+
<dependency>
79+
<groupId>ch.qos.logback</groupId>
80+
<artifactId>logback-classic</artifactId>
81+
</dependency>
7882
</dependencies>
7983
</project>

systests/rs-sse/rs-sse-base/src/main/java/org/apache/cxf/systest/jaxrs/sse/AbstractSseTest.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@
3636
import com.fasterxml.jackson.core.JsonProcessingException;
3737
import com.fasterxml.jackson.jakarta.rs.json.JacksonJsonProvider;
3838

39+
import ch.qos.logback.classic.Level;
40+
import ch.qos.logback.classic.Logger;
41+
import ch.qos.logback.classic.spi.ILoggingEvent;
42+
import ch.qos.logback.classic.spi.IThrowableProxy;
43+
import ch.qos.logback.core.read.ListAppender;
3944
import jakarta.ws.rs.client.Entity;
4045
import jakarta.ws.rs.client.WebTarget;
4146
import jakarta.ws.rs.core.HttpHeaders;
@@ -46,6 +51,8 @@
4651
import jakarta.ws.rs.sse.InboundSseEvent;
4752
import jakarta.ws.rs.sse.SseEventSource;
4853
import jakarta.ws.rs.sse.SseEventSource.Builder;
54+
import org.slf4j.LoggerFactory;
55+
4956

5057
import org.junit.Before;
5158
import org.junit.Test;
@@ -61,6 +68,10 @@
6168
import static org.junit.Assert.fail;
6269

6370

71+
72+
73+
74+
6475
public abstract class AbstractSseTest extends AbstractSseBaseTest {
6576
@Before
6677
public void setUp() {
@@ -71,6 +82,9 @@ public void setUp() {
7182

7283
}
7384

85+
86+
87+
7488
@Test
7589
public void testBooksStreamIsReturnedFromLastEventId() throws InterruptedException {
7690
final WebTarget target = createWebTarget("/rest/api/bookstore/sse/" + UUID.randomUUID())
@@ -409,7 +423,98 @@ public void testBooksSseContainerResponseAddedHeaders() throws InterruptedExcept
409423
assertThat(response.getHeaderString("X-My-ProtocolHeader"), equalTo("protocol-headers"));
410424
}
411425
}
426+
427+
428+
@Test
429+
public void testSseEndpointExceptionIsLoggedToConsole() throws Exception {
430+
final Logger logger = (Logger) LoggerFactory.getLogger("org.apache.cxf.jaxrs.JAXRSInvoker");
431+
432+
final Level oldLevel = logger.getLevel();
433+
final ListAppender<ILoggingEvent> appender = new ListAppender<>();
434+
appender.start();
435+
436+
try {
437+
logger.setLevel(Level.ERROR);
438+
logger.addAppender(appender);
439+
440+
try (Response r = createWebTarget("/rest/api/bookstore/sse/fail/request")
441+
.request(MediaType.SERVER_SENT_EVENTS)
442+
.get()) {
443+
// Force the client to actually start consuming
444+
r.readEntity(String.class);
445+
} catch (Exception ex) {
446+
// expected
447+
}
448+
449+
// Wait until we have at least one ERROR from JAXRSInvoker
450+
awaitEvents(2000, appender.list, 1);
451+
452+
assertTrue("Expected SSE log event, got:\n" + dump(appender),
453+
hasUnhandledExceptionEvent(appender));
454+
455+
assertTrue("Expected SSE marker in throwable, got:\n" + dump(appender),
456+
hasMarkerInUnhandledExceptionEvent(appender, "CXF-9189-MARKER"));
457+
} finally {
458+
logger.detachAppender(appender);
459+
logger.setLevel(oldLevel);
460+
appender.stop();
461+
}
462+
}
463+
464+
private static boolean hasUnhandledExceptionEvent(ListAppender<ILoggingEvent> appender) {
465+
final String msgNeedle = "Unhandled exception from JAX-RS invocation (async/SSE path)";
466+
for (ILoggingEvent e : appender.list) {
467+
String msg = e.getFormattedMessage();
468+
if (msg != null && msg.contains(msgNeedle)) {
469+
return true;
470+
}
471+
}
472+
return false;
473+
}
474+
475+
private static boolean hasMarkerInUnhandledExceptionEvent(ListAppender<ILoggingEvent> appender, String marker) {
476+
final String msgNeedle = "Unhandled exception from JAX-RS invocation (async/SSE path)";
477+
for (ILoggingEvent e : appender.list) {
478+
String msg = e.getFormattedMessage();
479+
if (msg == null || !msg.contains(msgNeedle)) {
480+
continue;
481+
}
482+
// marker can be in message OR in throwable chain
483+
if (msg.contains(marker) || throwableChainContains(e.getThrowableProxy(), marker)) {
484+
return true;
485+
}
486+
}
487+
return false;
488+
}
412489

490+
private static boolean throwableChainContains(IThrowableProxy tp, String needle) {
491+
for (IThrowableProxy cur = tp; cur != null; cur = cur.getCause()) {
492+
String m = cur.getMessage();
493+
if (m != null && m.contains(needle)) {
494+
return true;
495+
}
496+
}
497+
return false;
498+
}
499+
500+
private static String dump(ListAppender<ILoggingEvent> appender) {
501+
StringBuilder sb = new StringBuilder();
502+
for (ILoggingEvent e : appender.list) {
503+
sb.append('[').append(e.getLevel()).append("] ")
504+
.append(e.getLoggerName()).append(" - ")
505+
.append(e.getFormattedMessage());
506+
if (e.getThrowableProxy() != null) {
507+
sb.append(" (thrown: ")
508+
.append(e.getThrowableProxy().getClassName())
509+
.append(": ")
510+
.append(e.getThrowableProxy().getMessage())
511+
.append(')');
512+
}
513+
sb.append('\n');
514+
}
515+
return sb.toString();
516+
}
517+
413518
/**
414519
* Jetty / Undertow do not propagate errors from the runnable passed to
415520
* AsyncContext::start() up to the AsyncEventListener::onError(). Tomcat however
@@ -427,4 +532,6 @@ private static Consumer<InboundSseEvent> collect(final Collection<Book> books) {
427532
private static Consumer<InboundSseEvent> collectRaw(final Collection<String> titles) {
428533
return event -> titles.add(event.readData(String.class, MediaType.TEXT_PLAIN_TYPE));
429534
}
535+
536+
430537
}

systests/rs-sse/rs-sse-base/src/main/java/org/apache/cxf/systest/jaxrs/sse/BookStore.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,15 @@ public int filteredStats() {
236236
return BookStoreResponseFilter.getInvocations();
237237
}
238238

239+
@GET
240+
@Path("/sse/fail/request")
241+
@Produces(MediaType.SERVER_SENT_EVENTS)
242+
public void failOnRequestThread(@Context SseEventSink sink) {
243+
throw new RuntimeException("CXF-9189-MARKER: exception from SSE resource method should be logged");
244+
}
245+
246+
247+
239248
@PUT
240249
@Path("/filtered/stats")
241250
public void clearStats() {

systests/rs-sse/rs-sse-base/src/main/java/org/apache/cxf/systest/jaxrs/sse/BookStore2.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,16 @@ public void run() {
218218
public int filteredStats() {
219219
return BookStoreResponseFilter.getInvocations();
220220
}
221+
222+
@GET
223+
@Path("/sse/fail/request")
224+
@Produces(MediaType.SERVER_SENT_EVENTS)
225+
public void failOnRequestThread(@Context SseEventSink sink) {
226+
throw new RuntimeException("CXF-9189-MARKER: exception from SSE resource method should be logged");
227+
}
228+
229+
230+
221231

222232
@PUT
223233
@Path("/filtered/stats")

0 commit comments

Comments
 (0)