Skip to content

Commit

Permalink
Refactoring of sending HTTP 500 in ContainerMapper
Browse files Browse the repository at this point in the history
- previous version had too wide try-catch blocks where in one of deployment_all
  test cases one exception caused another which was preventable.
- this version logs just the original exception and then a message if it cannot
  send the http 500 or another exception if it tried and it failed, but no NPE.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
  • Loading branch information
dmatej committed Nov 14, 2022
1 parent 7a2308b commit c1f25ee
Showing 1 changed file with 32 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import org.glassfish.internal.grizzly.ContextMapper;
import org.glassfish.kernel.KernelLoggerInfo;

import static java.util.logging.Level.WARNING;

/**
* Container's mapper which maps {@link ByteBuffer} bytes representation to an {@link HttpHandler}, {@link
* ApplicationContainer} and ProtocolFilter chain. The mapping result is stored inside {@link MappingData} which
Expand Down Expand Up @@ -140,31 +142,42 @@ protected void configureMapper() {
public void service(final Request request, final Response response) throws Exception {
try {
request.addAfterServiceListener(afterServiceListener);

final Callable handler = lookupHandler(request, response);
handler.call();
lookupHandler(request, response).call();
} catch (Exception ex) {
try {
if (LOGGER.isLoggable(Level.WARNING)) {
final Object url = request.getRequest() == null ? null
: request.getRequest().getRequestURIRef().getDecodedRequestURIBC();
LogHelper.log(LOGGER, Level.WARNING, KernelLoggerInfo.exceptionMapper, ex, url);
}
logAndSendError(request, response, ex);
}
}

response.sendError(500);
} catch (Exception ex2) {
LOGGER.log(Level.WARNING, KernelLoggerInfo.exceptionMapper2, ex2);
if (ex2 instanceof CharConversionException) {
response.sendError(500);
}
}
private void logAndSendError(final Request request, final Response response, Exception ex) {
if (LOGGER.isLoggable(WARNING)) {
final Object url = toUrlForLogging(request);
LogHelper.log(LOGGER, WARNING, KernelLoggerInfo.exceptionMapper, ex, url);
}
if (response.getResponse() == null) {
LOGGER.log(WARNING, "Response is not set in {0}, there's nothing we can do now.", response);
return;
}
try {
response.sendError(500);
} catch (Exception ex2) {
LOGGER.log(WARNING, KernelLoggerInfo.exceptionMapper2, ex2);
}
}


private Object toUrlForLogging(final Request request) {
try {
return request.getRequest() == null ? null : request.getRequest().getRequestURIRef().getDecodedRequestURIBC();
} catch (CharConversionException e) {
return null;
}
}

private Callable lookupHandler(final Request request,
final Response response) throws CharConversionException, Exception {
MappingData mappingData;

private Callable lookupHandler(final Request request, final Response response)
throws CharConversionException, Exception {

MappingData mappingData;
mapperLock.readLock().lock();

try {
Expand Down

0 comments on commit c1f25ee

Please sign in to comment.