Add suppressed failures in Callback failed (#11435)

If an exception is thrown during failure handling, then record the original failure as a suppressed Throwable on the thrown exception
This commit is contained in:
Greg Wilkins 2024-02-29 20:06:50 +01:00 committed by GitHub
parent 56e05a973f
commit 4155e7bc25
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 193 additions and 88 deletions

View File

@ -1114,7 +1114,7 @@ public class HttpChannelState implements HttpChannel, Components
} }
if (writeCallback == null) if (writeCallback == null)
return null; return null;
return () -> writeCallback.failed(x); return () -> HttpChannelState.failed(writeCallback, x);
} }
public long getContentBytesWritten() public long getContentBytesWritten()
@ -1229,7 +1229,7 @@ public class HttpChannelState implements HttpChannel, Components
if (writeFailure != null) if (writeFailure != null)
{ {
Throwable failure = writeFailure; Throwable failure = writeFailure;
httpChannelState._writeInvoker.run(() -> callback.failed(failure)); httpChannelState._writeInvoker.run(() -> HttpChannelState.failed(callback, failure));
return; return;
} }
@ -1297,7 +1297,7 @@ public class HttpChannelState implements HttpChannel, Components
httpChannel.lockedStreamSendCompleted(false); httpChannel.lockedStreamSendCompleted(false);
} }
if (callback != null) if (callback != null)
httpChannel._writeInvoker.run(() -> callback.failed(x)); httpChannel._writeInvoker.run(() -> HttpChannelState.failed(callback, x));
} }
@Override @Override
@ -1512,6 +1512,8 @@ public class HttpChannelState implements HttpChannel, Components
*/ */
@Override @Override
public void failed(Throwable failure) public void failed(Throwable failure)
{
try
{ {
// Called when the request/response cycle is completing with a failure. // Called when the request/response cycle is completing with a failure.
HttpStream stream; HttpStream stream;
@ -1549,6 +1551,12 @@ public class HttpChannelState implements HttpChannel, Components
else else
_request.getHttpChannelState()._handlerInvoker.failed(failure); _request.getHttpChannelState()._handlerInvoker.failed(failure);
} }
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, failure);
throw t;
}
}
private boolean lockedCompleteCallback() private boolean lockedCompleteCallback()
{ {
@ -1683,7 +1691,7 @@ public class HttpChannelState implements HttpChannel, Components
} }
else else
{ {
httpChannelState._handlerInvoker.failed(failure); HttpChannelState.failed(httpChannelState._handlerInvoker, failure);
} }
} }
@ -1705,9 +1713,8 @@ public class HttpChannelState implements HttpChannel, Components
httpChannelState = _request.lockedGetHttpChannelState(); httpChannelState = _request.lockedGetHttpChannelState();
httpChannelState._response._status = _errorResponse._status; httpChannelState._response._status = _errorResponse._status;
} }
if (ExceptionUtil.areNotAssociated(failure, x)) ExceptionUtil.addSuppressedIfNotAssociated(failure, x);
failure.addSuppressed(x); HttpChannelState.failed(httpChannelState._handlerInvoker, failure);
httpChannelState._handlerInvoker.failed(failure);
} }
@Override @Override
@ -1735,10 +1742,18 @@ public class HttpChannelState implements HttpChannel, Components
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{
try
{ {
completing(); completing();
super.failed(x); super.failed(x);
} }
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, x);
throw t;
}
}
private void completing() private void completing()
{ {
@ -1868,4 +1883,25 @@ public class HttpChannelState implements HttpChannel, Components
} }
} }
} }
/**
* Invoke a callback failure, handling any {@link Throwable} thrown
* by adding the passed {@code failure} as a suppressed with
* {@link ExceptionUtil#addSuppressedIfNotAssociated(Throwable, Throwable)}.
* @param callback The callback to fail
* @param failure The failure
* @throws RuntimeException If thrown, will have the {@code failure} added as a suppressed.
*/
private static void failed(Callback callback, Throwable failure)
{
try
{
callback.failed(failure);
}
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, failure);
throw t;
}
}
} }

View File

@ -120,9 +120,17 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{
try
{ {
completable.completeExceptionally(x); completable.completeExceptionally(x);
} }
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, x);
throw t;
}
}
@Override @Override
public InvocationType getInvocationType() public InvocationType getInvocationType()
@ -164,9 +172,17 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{
try
{ {
failure.accept(x); failure.accept(x);
} }
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, x);
throw t;
}
}
@Override @Override
public InvocationType getInvocationType() public InvocationType getInvocationType()
@ -272,14 +288,7 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{ {
try Callback.failed(callback::failed, completed, x);
{
callback.failed(x);
}
finally
{
completed.accept(x);
}
} }
}; };
} }
@ -307,22 +316,19 @@ public interface Callback extends Invocable
} }
catch (Throwable t) catch (Throwable t)
{ {
callback.failed(t); Callback.failed(callback, t);
} }
} }
private void completed(Throwable ignored)
{
completed.run();
}
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{ {
try Callback.failed(this::completed, callback::failed, x);
{
completed.run();
}
catch (Throwable t)
{
x.addSuppressed(t);
}
callback.failed(x);
} }
}; };
} }
@ -348,8 +354,8 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{ {
cause.addSuppressed(x); ExceptionUtil.addSuppressedIfNotAssociated(cause, x);
callback.failed(cause); Callback.failed(callback, cause);
} }
}; };
} }
@ -380,9 +386,17 @@ public interface Callback extends Invocable
@Override @Override
default void failed(Throwable x) default void failed(Throwable x)
{
try
{ {
completed(); completed();
} }
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, x);
throw t;
}
}
} }
/** /**
@ -408,6 +422,11 @@ public interface Callback extends Invocable
{ {
} }
private void completed(Throwable ignored)
{
completed();
}
@Override @Override
public void succeeded() public void succeeded()
{ {
@ -424,14 +443,7 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{ {
try Callback.failed(callback::failed, this::completed, x);
{
callback.failed(x);
}
finally
{
completed();
}
} }
@Override @Override
@ -472,18 +484,7 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{ {
try Callback.failed(cb1::failed, cb2::failed, x);
{
cb1.failed(x);
}
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(x, t);
}
finally
{
cb2.failed(x);
}
} }
@Override @Override
@ -533,8 +534,7 @@ public interface Callback extends Invocable
@Override @Override
public void failed(Throwable x) public void failed(Throwable x)
{ {
callback.failed(x); Callback.failed(callback::failed, super::failed, x);
super.failed(x);
} }
}; };
} }
@ -592,4 +592,64 @@ public interface Callback extends Invocable
return completable; return completable;
} }
} }
/**
* Invoke a callback failure, handling any {@link Throwable} thrown
* by adding the passed {@code failure} as a suppressed with
* {@link ExceptionUtil#addSuppressedIfNotAssociated(Throwable, Throwable)}.
* @param callback The callback to fail
* @param failure The failure
* @throws RuntimeException If thrown, will have the {@code failure} added as a suppressed.
*/
private static void failed(Callback callback, Throwable failure)
{
try
{
callback.failed(failure);
}
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, failure);
throw t;
}
}
/**
* Invoke two consumers of a failure, handling any {@link Throwable} thrown
* by adding the passed {@code failure} as a suppressed with
* {@link ExceptionUtil#addSuppressedIfNotAssociated(Throwable, Throwable)}.
* @param first The first consumer of a failure
* @param second The first consumer of a failure
* @param failure The failure
* @throws RuntimeException If thrown, will have the {@code failure} added as a suppressed.
*/
private static void failed(Consumer<Throwable> first, Consumer<Throwable> second, Throwable failure)
{
// This is an improved version of:
// try
// {
// first.accept(failure);
// }
// finally
// {
// second.accept(failure);
// }
try
{
first.accept(failure);
}
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(failure, t);
}
try
{
second.accept(failure);
}
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, failure);
throw t;
}
}
} }

View File

@ -499,6 +499,9 @@ public class ServletChannel
ExceptionUtil.addSuppressedIfNotAssociated(cause, x); ExceptionUtil.addSuppressedIfNotAssociated(cause, x);
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug("Could not perform error handling, aborting", cause); LOG.debug("Could not perform error handling, aborting", cause);
try
{
if (_state.isResponseCommitted()) if (_state.isResponseCommitted())
{ {
// Perform the same behavior as when the callback is failed. // Perform the same behavior as when the callback is failed.
@ -510,6 +513,12 @@ public class ServletChannel
sendErrorResponseAndComplete(); sendErrorResponseAndComplete();
} }
} }
catch (Throwable t)
{
ExceptionUtil.addSuppressedIfNotAssociated(t, cause);
throw t;
}
}
finally finally
{ {
// clean up the context that was set in Response.sendError // clean up the context that was set in Response.sendError