Skip to content

Commit

Permalink
Exceptions from fetch handlers should wait for output gate.
Browse files Browse the repository at this point in the history
Prior to this, if an application queued a storage write, then threw an exception, and the storage write failed later on, the exception might nevertheless be received by the caller. If, for some reason, an application were confirming writes by throwing exceptions, this could result in confirming a write that didn't actually make it to disk.

Of course, surely no app does that. The bigger problem is that this could screw with the `durableObjectReset` flag on exceptions thrown after storage breakages. Say a storage write fails, and we respond by simultaneously throwing an exception from the storage API to the application, as well as breaking the output gate with the same exception. The version of the exception thrown at the application could propagate out of the fetch handler before the output gate had a chance to propagate its version. However, the version that throws *through* the application would lose the `broken.` annotation, as this by design does not transit through application code. The version of the exception that broke the output gate keeps this annotation, so is a better one to throw to the client. By properly waiting for the output gate here, we can make sure the preferred exception is propagated.
  • Loading branch information
kentonv committed Apr 28, 2023
1 parent af0c339 commit 8b89221
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/workerd/io/worker-entrypoint.c++
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,12 @@ kj::Promise<void> WorkerEntrypoint::request(
loggedExceptionEarlier = true;
context.logUncaughtExceptionAsync(UncaughtExceptionSource::REQUEST_HANDLER,
kj::cp(exception));
return kj::mv(exception);

// Do not allow the exception to escape the isolate without waiting for the output gate to
// open. Note that in the success path, this is taken care of in `FetchEvent::respondWith()`.
return context.waitForOutputLocks()
.then([exception = kj::mv(exception)]() mutable
-> kj::Promise<void> { return kj::mv(exception); });
}).attach(kj::defer([this,incomingRequest = kj::mv(incomingRequest),&context]() mutable {
// The request has been canceled, but allow it to continue executing in the background.
if (context.isFailOpen()) {
Expand Down

0 comments on commit 8b89221

Please # to comment.