Skip to content

Commit

Permalink
Call CompletionCallback just once with DefaultExceptionMapper
Browse files Browse the repository at this point in the history
Do not propagate error to the client

Signed-off-by: jansupol <jan.supol@oracle.com>
  • Loading branch information
jansupol committed Jun 8, 2022
1 parent 54a2854 commit abd1a43
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2022 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -19,6 +19,7 @@
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import org.glassfish.jersey.server.internal.LocalizationMessages;

class DefaultExceptionMapper implements ExceptionMapper<Throwable> {
@Override
Expand All @@ -35,6 +36,8 @@ private static Response processWebApplicationException(WebApplicationException e
}

private static Response processDefaultException(Throwable exception) {
return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(exception.getMessage()).build();
return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
.entity(LocalizationMessages.ERROR_EXCEPTION_MAPPING_THROWN_TO_CONTAINER())
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,11 @@ public Responder(final RequestProcessingContext processingContext, final ServerR

public void process(ContainerResponse response) {
processingContext.monitoringEventBuilder().setContainerResponse(response);
response = processResponse(response);
response = processResponse(response, null);
release(response);
}

private ContainerResponse processResponse(ContainerResponse response) {
private ContainerResponse processResponse(ContainerResponse response, Throwable unmappedThrowable) {
final Stage<ContainerResponse> respondingRoot = processingContext.createRespondingRoot();

if (respondingRoot != null) {
Expand All @@ -379,7 +379,7 @@ private ContainerResponse processResponse(ContainerResponse response) {

// no-exception zone
// the methods below are guaranteed to not throw any exceptions
completionCallbackRunner.onComplete(null);
completionCallbackRunner.onComplete(unmappedThrowable);
return response;
}

Expand Down Expand Up @@ -427,7 +427,7 @@ public void process(final Throwable throwable) {
final Response exceptionResponse = mapException(throwable);
try {
response = preProcessResponse(exceptionResponse, request);
processResponse(response);
processResponse(response, null);
} catch (final Throwable respError) {
LOGGER.log(Level.SEVERE, LocalizationMessages.ERROR_PROCESSING_RESPONSE_FROM_ALREADY_MAPPED_EXCEPTION());
processingContext.monitoringEventBuilder()
Expand All @@ -448,9 +448,10 @@ public void process(final Throwable throwable) {
try {
request.getResponseWriter().failure(responseError);
} finally {
completionCallbackRunner.onComplete(responseError);

defaultMapperResponse = processResponseWithDefaultExceptionMapper(responseError, request);

// completionCallbackRunner.onComplete(responseError); is called from
// processResponseWithDefaultExceptionMapper
}

}
Expand Down Expand Up @@ -488,7 +489,7 @@ private boolean processResponseError(final Throwable responseError) {

if (processedError != null) {
processedResponse =
processResponse(new ContainerResponse(processingContext.request(), processedError));
processResponse(new ContainerResponse(processingContext.request(), processedError), null);
processed = true;
}
} catch (final Throwable throwable) {
Expand Down Expand Up @@ -545,10 +546,7 @@ private Response mapException(final Throwable originalThrowable) throws Throwabl

final long timestamp = tracingLogger.timestamp(ServerTraceEvent.EXCEPTION_MAPPING);
final ExceptionMapper mapper = runtime.exceptionMappers.findMapping(throwable);
if (mapper != null
&& !DefaultExceptionMapper.class.getName()
.equals(mapper.getClass().getName())
) {
if (mapper != null && !DefaultExceptionMapper.class.isInstance(mapper)) {
return processExceptionWithMapper(mapper, throwable, timestamp);
}
if (waeResponse != null) {
Expand Down Expand Up @@ -625,7 +623,7 @@ private ContainerResponse processResponseWithDefaultExceptionMapper(Throwable ex
ContainerRequest request) {
long timestamp = tracingLogger.timestamp(ServerTraceEvent.EXCEPTION_MAPPING);
final Response response = processExceptionWithMapper(DEFAULT_EXCEPTION_MAPPER, exception, timestamp);
return processResponse(preProcessResponse(response, request));
return processResponse(preProcessResponse(response, request), exception);
}

private ContainerResponse writeResponse(final ContainerResponse response) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright (c) 2022 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
* version 2 with the GNU Classpath Exception, which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package org.glassfish.jersey.server;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.container.AsyncResponse;
import jakarta.ws.rs.container.CompletionCallback;
import jakarta.ws.rs.container.Suspended;
import jakarta.ws.rs.core.Response;
import org.junit.Assert;
import org.junit.Test;

import java.io.IOException;
import java.util.concurrent.atomic.AtomicInteger;

public class DefaultExceptionMapperTest {
public static final String MESSAGE = "DefaultExceptionMapperTest I/O Exception";
@Test
public void testIOException() {
IOException ioe = new IOException(MESSAGE);
DefaultExceptionMapper mapper = new DefaultExceptionMapper();
Response response = mapper.toResponse(ioe);
Assert.assertFalse(response.getEntity().toString().contains(MESSAGE));
}

@Test
public void testCompletionCallback() {
AtomicInteger counter = new AtomicInteger();
CompletionCallback hitOnceCallback = new CompletionCallback() {
@Override
public void onComplete(Throwable throwable) {
counter.incrementAndGet();
}
};
ResourceConfig resourceConfig = new ResourceConfig().register(new IOExThrowingResource(hitOnceCallback));
ApplicationHandler applicationHandler = new ApplicationHandler(resourceConfig);
try {
applicationHandler.apply(RequestContextBuilder.from("/", "GET").build()).get();
} catch (Exception e) {
// expected
}

Assert.assertEquals(1, counter.get());
}

@Path("/")
public static class IOExThrowingResource {
private final CompletionCallback callback;

public IOExThrowingResource(CompletionCallback callback) {
this.callback = callback;
}

@GET
public String doGet(@Suspended AsyncResponse asyncResponse) throws IOException {
asyncResponse.register(callback);
throw new IOException(MESSAGE);
}
}
}

0 comments on commit abd1a43

Please sign in to comment.