diff --git a/src/workerd/api/BUILD.bazel b/src/workerd/api/BUILD.bazel index 04df1378abe..697db5d1458 100644 --- a/src/workerd/api/BUILD.bazel +++ b/src/workerd/api/BUILD.bazel @@ -418,6 +418,7 @@ wd_cc_library( srcs = ["util.c++"], hdrs = ["util.h"], implementation_deps = [ + "//src/workerd/util:exception", "//src/workerd/util:mimetype", "//src/workerd/util:strings", "@simdutf", diff --git a/src/workerd/api/crypto/crypto.c++ b/src/workerd/api/crypto/crypto.c++ index f17b69f397e..5c98b5c63f3 100644 --- a/src/workerd/api/crypto/crypto.c++ +++ b/src/workerd/api/crypto/crypto.c++ @@ -806,13 +806,17 @@ DigestStream::DigestStream(kj::Own controller, state(Ready(kj::mv(algorithm), kj::mv(resolver))) {} void DigestStream::dispose(jsg::Lock& js) { - js.tryCatch([&] { + JSG_TRY { KJ_IF_SOME(ready, state.tryGet()) { auto reason = js.typeError("The DigestStream was disposed."); ready.resolver.reject(js, reason); state.init(js.v8Ref(reason)); } - }, [&](jsg::Value exception) { js.throwException(kj::mv(exception)); }); + } + catch (...) { + jsg::Value exception = getCaughtExceptionAsJsg(); + js.throwException(kj::mv(exception)); + } } void DigestStream::visitForMemoryInfo(jsg::MemoryTracker& tracker) const { diff --git a/src/workerd/api/memory-cache.c++ b/src/workerd/api/memory-cache.c++ index 703e23bf47d..22b2652f192 100644 --- a/src/workerd/api/memory-cache.c++ +++ b/src/workerd/api/memory-cache.c++ @@ -430,11 +430,13 @@ void SharedMemoryCache::Use::delete_(const kj::String& key) const { // Attempts to serialize a JavaScript value. If that fails, this function throws // a tunneled exception, see jsg::createTunneledException(). static kj::Own hackySerialize(jsg::Lock& js, jsg::JsRef& value) { - return js.tryCatch([&]() -> kj::Own { + JSG_TRY { jsg::Serializer serializer(js); serializer.write(js, value.getHandle(js)); return kj::atomicRefcounted(serializer.release().data); - }, [&](jsg::Value&& exception) -> kj::Own { + } + catch (...) { + jsg::Value exception = getCaughtExceptionAsJsg(); // We run into big problems with tunneled exceptions here. When // the toString() function of the JavaScript error is not marked // as side effect free, tunneling the exception fails entirely @@ -450,7 +452,7 @@ static kj::Own hackySerialize(jsg::Lock& js, jsg::JsRef> MemoryCache::read(jsg::Lock& js, diff --git a/src/workerd/api/messagechannel.c++ b/src/workerd/api/messagechannel.c++ index 7313a35c254..788a778dc0a 100644 --- a/src/workerd/api/messagechannel.c++ +++ b/src/workerd/api/messagechannel.c++ @@ -42,17 +42,18 @@ MessagePort::MessagePort() } void MessagePort::dispatchMessage(jsg::Lock& js, const jsg::JsValue& value) { - js.tryCatch([&] { + JSG_TRY_CATCH(getMessageException) try { auto message = js.alloc(js, kj::str("message"), value, kj::String(), JSG_THIS); dispatchEventImpl(js, kj::mv(message)); - }, [&](jsg::Value exception) { + } catch (...) { // There was an error dispatching the message event. // We will dispatch a messageerror event instead. + jsg::Value exception = getMessageException(); auto message = js.alloc( js, kj::str("message"), jsg::JsValue(exception.getHandle(js)), kj::String(), JSG_THIS); dispatchEventImpl(js, kj::mv(message)); // Now, if this dispatchEventImpl throws, we just blow up. Don't try to catch it. - }); + } } // Deliver the message to this port, buffering if necessary if the port diff --git a/src/workerd/api/streams/standard.c++ b/src/workerd/api/streams/standard.c++ index 8705fb45d7c..c70a4396ac2 100644 --- a/src/workerd/api/streams/standard.c++ +++ b/src/workerd/api/streams/standard.c++ @@ -505,31 +505,46 @@ jsg::Promise maybeRunAlgorithm( // throws synchronously, we have to convert that synchronous throw // into a proper rejected jsg::Promise. KJ_IF_SOME(algorithm, maybeAlgorithm) { - // We need two layers of tryCatch here, unfortunately. The inner layer + // We need two layers of JSG_TRY here, unfortunately. The inner layer // covers the algorithm implementation itself and is our typical error // handling path. It ensures that if the algorithm throws an exception, // that is properly converted in to a rejected promise that is *then* - // handled by the onFailure handler that is passed in. The outer tryCatch + // handled by the onFailure handler that is passed in. The outer JSG_TRY // handles the rare and generally unexpected failure of the calls to // .then() itself, which can throw JS exceptions synchronously in certain // rare cases. For those we return a rejected promise but do not call the // onFailure case since such errors are generally indicative of a fatal // condition in the isolate (e.g. out of memory, other fatal exception, etc). - return js.tryCatch([&] { + JSG_TRY { KJ_IF_SOME(ioContext, IoContext::tryCurrent()) { - return js - .tryCatch([&] { return algorithm(js, kj::fwd(args)...); }, - [&](jsg::Value&& exception) { return js.rejectedPromise(kj::mv(exception)); }) - .then(js, ioContext.addFunctor(kj::mv(onSuccess)), - ioContext.addFunctor(kj::mv(onFailure))); + auto getInnerPromise = [&]() -> jsg::Promise { + JSG_TRY { + return algorithm(js, kj::fwd(args)...); + } + catch (...) { + jsg::Value exception = getCaughtExceptionAsJsg(); + return js.rejectedPromise(kj::mv(exception)); + } + }; + return getInnerPromise().then( + js, ioContext.addFunctor(kj::mv(onSuccess)), ioContext.addFunctor(kj::mv(onFailure))); } else { - return js - .tryCatch([&] { return algorithm(js, kj::fwd(args)...); }, - [&](jsg::Value&& exception) { - return js.rejectedPromise(kj::mv(exception)); - }).then(js, kj::mv(onSuccess), kj::mv(onFailure)); + auto getInnerPromise = [&]() -> jsg::Promise { + JSG_TRY { + return algorithm(js, kj::fwd(args)...); + } + catch (...) { + jsg::Value exception = getCaughtExceptionAsJsg(); + return js.rejectedPromise(kj::mv(exception)); + } + }; + return getInnerPromise().then(js, kj::mv(onSuccess), kj::mv(onFailure)); } - }, [&](jsg::Value&& exception) { return js.rejectedPromise(kj::mv(exception)); }); + } + catch (...) { + jsg::Value exception = getCaughtExceptionAsJsg(); + return js.rejectedPromise(kj::mv(exception)); + } } // If the algorithm does not exist, we just handle it as a success and move on. @@ -1529,10 +1544,11 @@ jsg::Promise WritableImpl::write( size_t size = 1; KJ_IF_SOME(sizeFunc, algorithms.size) { kj::Maybe failure; - js.tryCatch([&] { size = sizeFunc(js, value); }, [&](jsg::Value exception) { + JSG_TRY_CATCH(getSizeFuncException) try { size = sizeFunc(js, value); } catch (...) { + auto exception = getSizeFuncException(); startErroring(js, self.addRef(), exception.getHandle(js)); failure = kj::mv(exception); - }); + } KJ_IF_SOME(exception, failure) { return js.rejectedPromise(kj::mv(exception)); } diff --git a/src/workerd/api/urlpattern-standard.c++ b/src/workerd/api/urlpattern-standard.c++ index 2fd30c158c0..289a2088555 100644 --- a/src/workerd/api/urlpattern-standard.c++ +++ b/src/workerd/api/urlpattern-standard.c++ @@ -16,9 +16,12 @@ std::optional URLPattern::URLPatt flags | static_cast(jsg::Lock::RegExpFlags::kIGNORE_CASE)); } - return js.tryCatch([&]() -> std::optional { + JSG_TRY { return jsg::JsRef(js, js.regexp(kj::StringPtr(pattern.data(), pattern.size()), flags)); - }, [&](auto reason) -> std::optional { return std::nullopt; }); + } + catch (...) { + return std::nullopt; + } } bool URLPattern::URLPatternRegexEngine::regex_match( diff --git a/src/workerd/api/urlpattern.c++ b/src/workerd/api/urlpattern.c++ index 0cbe5a141f2..95631f06fd2 100644 --- a/src/workerd/api/urlpattern.c++ +++ b/src/workerd/api/urlpattern.c++ @@ -13,16 +13,17 @@ namespace workerd::api { namespace { jsg::JsRef compileRegex( jsg::Lock& js, const jsg::UrlPattern::Component& component, bool ignoreCase) { - return js.tryCatch([&] { + JSG_TRY { jsg::Lock::RegExpFlags flags = jsg::Lock::RegExpFlags::kUNICODE; if (ignoreCase) { flags = static_cast( flags | static_cast(jsg::Lock::RegExpFlags::kIGNORE_CASE)); } return jsg::JsRef(js, js.regexp(component.getRegex(), flags)); - }, [&](auto reason) -> jsg::JsRef { + } + catch (...) { JSG_FAIL_REQUIRE(TypeError, "Invalid regular expression syntax."); - }); + } } jsg::Ref create(jsg::Lock& js, jsg::UrlPattern pattern) { diff --git a/src/workerd/api/util.c++ b/src/workerd/api/util.c++ index d6bad558aa8..9189ac05148 100644 --- a/src/workerd/api/util.c++ +++ b/src/workerd/api/util.c++ @@ -6,6 +6,7 @@ #include "simdutf.h" +#include #include #include @@ -69,10 +70,10 @@ namespace { template auto translateTeeErrors(Func&& f) -> decltype(kj::fwd(f)()) { - try { + KJ_TRY { co_return co_await f(); - } catch (...) { - auto exception = kj::getCaughtExceptionAsKj(); + } + KJ_CATCH(kj::Exception & exception) { KJ_IF_SOME(e, translateKjException(exception, { diff --git a/src/workerd/jsg/function-test.c++ b/src/workerd/jsg/function-test.c++ index 4761eeb9c0d..3be495c18a4 100644 --- a/src/workerd/jsg/function-test.c++ +++ b/src/workerd/jsg/function-test.c++ @@ -191,10 +191,27 @@ struct FunctionContext: public ContextGlobalObject { }); } + kj::String testTryCatch2(Lock& js, jsg::Function thrower) { + // Here we prove that the macro is if-else friendly. + // Note that clang-format doesn't recognize it as a `try`, so we get wonky formatting. Oh well. + if (true) JSG_TRY { + return kj::str(thrower(js)); + } + catch (...) { + Value exception = getCaughtExceptionAsJsg(); + auto handle = exception.getHandle(js); + return kj::str("caught: ", handle); + } + else { + KJ_UNREACHABLE; + } + } + JSG_RESOURCE_TYPE(FunctionContext) { JSG_METHOD(test); JSG_METHOD(test2); JSG_METHOD(testTryCatch); + JSG_METHOD(testTryCatch2); JSG_READONLY_PROTOTYPE_PROPERTY(square, getSquare); JSG_READONLY_PROTOTYPE_PROPERTY(gcLambda, getGcLambda); @@ -220,6 +237,9 @@ KJ_TEST("jsg::Function") { e.expectEval("testTryCatch(() => { return 123; })", "string", "123"); e.expectEval("testTryCatch(() => { throw new Error('foo'); })", "string", "caught: Error: foo"); + + e.expectEval("testTryCatch2(() => { return 123; })", "string", "123"); + e.expectEval("testTryCatch2(() => { throw new Error('foo'); })", "string", "caught: Error: foo"); } } // namespace diff --git a/src/workerd/jsg/jsg.h b/src/workerd/jsg/jsg.h index cab79368322..7a1d46a71f6 100644 --- a/src/workerd/jsg/jsg.h +++ b/src/workerd/jsg/jsg.h @@ -2995,6 +2995,123 @@ inline Value SelfRef::asValue(Lock& js) const { return Value(js.v8Isolate, getHandle(js).As()); } +namespace _ { + +// Helper class for converting caught exceptions to JSG Values in catch blocks. +// This is used internally by the JSG_TRY macro to provide clean exception handling +// that works with both JavaScript exceptions and KJ exceptions. +// +// The class automatically sets up a v8::TryCatch when constructed and can convert +// any caught exception to a jsg::Value when called. It handles: +// - JsExceptionThrown: Returns the V8 exception value directly +// - kj::Exception: Converts to JS using Lock::exceptionToJs() +// +// Usage is typically through JSG_TRY macro: +// JSG_TRY { +// someCodeThatMightThrow(); +// } +// catch (...) { +// auto jsException = getCaughtExceptionAsJsg(); +// // Handle the JS-converted exception +// } +// +// IMPORTANT: The operator() can only be called once per instance, as it consumes +// the internal TryCatch holder. Subsequent calls will throw. +// +// WARNING: Never call the exception converter function in the try block! +// It will fail with a raw `throw` because there's no active exception to re-throw. +// Only call it from within catch blocks where an exception has actually been caught. +class GetCaughtExceptionAsJsg { + public: + GetCaughtExceptionAsJsg() { + maybeHolder.emplace(Lock::current().v8Isolate); + } + + // Handle any in-flight JsExceptionThrown or kj::Exception. If a JsExceptionThrown is found, our + // v8::TryCatch's currently held exception value is returned. If a kj::Exception is found, it is + // converted to a JS value by passing it and `options` to `Lock::exceptionToJs()`. + Value operator()(ExceptionToJsOptions options = {}) { + // This function always consumes the holder. + KJ_DEFER(maybeHolder = kj::none); + auto& tryCatch = + KJ_REQUIRE_NONNULL(maybeHolder, "getCaughtExceptionAsJsg() can only be called once") + .tryCatch; + + // Same logic as that found in `jsg::Lock::tryCatch()`. + try { + throw; + } catch (JsExceptionThrown&) { + if (!tryCatch.CanContinue() || !tryCatch.HasCaught() || tryCatch.Exception().IsEmpty()) { + tryCatch.ReThrow(); + throw; + } + return Value(Lock::current().v8Isolate, tryCatch.Exception()); + } catch (kj::Exception& e) { + return Lock::current().exceptionToJs(kj::mv(e), options); + } + } + + private: + // Simple wrapper to work around v8::TryCatch's deleted operator new. + struct Holder { + v8::TryCatch tryCatch; + explicit Holder(v8::Isolate* isolate): tryCatch(isolate) {} + }; + + kj::Maybe maybeHolder; +}; + +} // namespace _ + +// General-purpose macro for introducing scoped variables into a statement. Uses the +// KJ_IF_SOME-style scoping trick where the variable is declared in an if condition that's always +// false, followed by an else that executes the actual statement. This allows the variable to be in +// scope for the entire statement, and more importantly to go out of scope at the end of the +// statement without needing braces. The scope is tied to the statement itself, not to a block. +// +// It is valid to use multiple KJ_STMT_SCOPED macros for a single statement. +// +// TODO(now): Move to libkj. +#define KJ_STMT_SCOPED(decl) \ + if (decl; false) { \ + } else + +// JSG_TRY_CATCH macro for exception handling with a custom-named exception converter. +// Unlike JSG_TRY, this doesn't include the `try` keyword, giving more explicit control. +// More transparent to colleagues and clang-format since it shows raw try-catch syntax. +// +// Usage: +// JSG_TRY_CATCH(getCaughtExceptionAsJsg) try { +// someCodeThatMightThrow(); +// } catch (...) { +// jsg::Value exception = getCaughtExceptionAsJsg(); +// // Handle the JS-converted exception +// } +// +// WARNING: Never call the exception converter function in the try block! +// It will fail with a raw `throw` because there's no active exception to re-throw. +// Only call it from within catch blocks where an exception has actually been caught. +#define JSG_TRY_CATCH(name) KJ_STMT_SCOPED(::workerd::jsg::_::GetCaughtExceptionAsJsg name) + +// JSG_TRY is a convenience macro implemented in terms of JSG_TRY_CATCH. +// It automatically names the exception converter `getCaughtExceptionAsJsg` and includes the `try` +// keyword. +// +// Note: This macro doesn't play well with clang-format due to the embedded `try` keyword. +// +// Usage: +// JSG_TRY { +// someCodeThatMightThrow(); +// } catch (...) { +// jsg::Value exception = getCaughtExceptionAsJsg(); +// // Handle the JS-converted exception +// } +// +// WARNING: Never call getCaughtExceptionAsJsg() in the try block! +// It will fail with a raw `throw` because there's no active exception to re-throw. +// Only call it from within catch blocks where an exception has actually been caught. +#define JSG_TRY JSG_TRY_CATCH(getCaughtExceptionAsJsg) try + } // namespace workerd::jsg // clang-format off diff --git a/src/workerd/jsg/modules-new.c++ b/src/workerd/jsg/modules-new.c++ index 48b57c9b483..1586c74c9c7 100644 --- a/src/workerd/jsg/modules-new.c++ +++ b/src/workerd/jsg/modules-new.c++ @@ -1701,25 +1701,31 @@ kj::ArrayPtr Module::ModuleNamespace::getNamedExports() con Module::EvaluateCallback Module::newTextModuleHandler(kj::ArrayPtr data) { return [data](Lock& js, const Url& id, const ModuleNamespace& ns, const CompilationObserver&) -> bool { - return js.tryCatch([&] { return ns.setDefault(js, js.str(data)); }, [&](Value exception) { + JSG_TRY { + return ns.setDefault(js, js.str(data)); + } + catch (...) { + Value exception = getCaughtExceptionAsJsg(); js.v8Isolate->ThrowException(exception.getHandle(js)); return false; - }); + } }; } Module::EvaluateCallback Module::newDataModuleHandler(kj::ArrayPtr data) { return [data](Lock& js, const Url& id, const ModuleNamespace& ns, const CompilationObserver&) -> bool { - return js.tryCatch([&] { + JSG_TRY { auto backing = jsg::BackingStore::alloc(js, data.size()); backing.asArrayPtr().copyFrom(data); auto buffer = jsg::BufferSource(js, kj::mv(backing)); return ns.setDefault(js, JsValue(buffer.getHandle(js))); - }, [&](Value exception) { + } + catch (...) { + Value exception = getCaughtExceptionAsJsg(); js.v8Isolate->ThrowException(exception.getHandle(js)); return false; - }); + } }; } diff --git a/src/workerd/server/BUILD.bazel b/src/workerd/server/BUILD.bazel index 63eaea3511b..9397a26681f 100644 --- a/src/workerd/server/BUILD.bazel +++ b/src/workerd/server/BUILD.bazel @@ -221,6 +221,7 @@ wd_cc_library( "//src/workerd/io:bundle-fs", "//src/workerd/io:worker-entrypoint", "//src/workerd/jsg", + "//src/workerd/util:exception", "//src/workerd/util:perfetto", "//src/workerd/util:websocket-error-handler", "@capnp-cpp//src/kj/compat:kj-gzip", diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++ index 14108fa7faa..1badf0913c4 100644 --- a/src/workerd/server/server.c++ +++ b/src/workerd/server/server.c++ @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -1429,11 +1430,11 @@ class Server::InspectorService final: public kj::HttpService, public kj::HttpSer KJ_LOG(INFO, kj::str("Inspector client attaching [", id, "]")); auto webSocket = response.acceptWebSocket(responseHeaders); kj::Duration timerOffset = 0 * kj::MILLISECONDS; - try { + KJ_TRY { co_return co_await ref->attachInspector( isolateThreadExecutor->addRef(), timer, timerOffset, *webSocket); - } catch (...) { - auto exception = kj::getCaughtExceptionAsKj(); + } + KJ_CATCH(kj::Exception & exception) { if (exception.getType() == kj::Exception::Type::DISCONNECTED) { // This likely just means that the inspector client was closed. // Nothing to do here but move along. @@ -1616,12 +1617,12 @@ class RequestObserverWithTracer final: public RequestObserver, public WorkerInte const kj::HttpHeaders& headers, kj::AsyncInputStream& requestBody, kj::HttpService::Response& response) override { - try { + KJ_TRY { SimpleResponseObserver responseWrapper(&fetchStatus, response); co_await KJ_ASSERT_NONNULL(inner).request(method, url, headers, requestBody, responseWrapper); - } catch (...) { + } + KJ_CATCH(kj::Exception & exception) { fetchStatus = 500; - auto exception = kj::getCaughtExceptionAsKj(); reportFailure(exception, FailureSource::OTHER); kj::throwFatalException(kj::mv(exception)); } diff --git a/src/workerd/util/exception.h b/src/workerd/util/exception.h index 4a3240a89d8..7106fa0c26d 100644 --- a/src/workerd/util/exception.h +++ b/src/workerd/util/exception.h @@ -12,3 +12,26 @@ namespace workerd { constexpr kj::Exception::DetailTypeId MEMORY_LIMIT_DETAIL_ID = 0xbaf76dd7ce5bd8cfull; } // namespace workerd + +// KJ exception handling macros that allow catching exceptions by type instead of using +// the awkward `catch (...)` + `kj::getCaughtExceptionAsKj()` pattern. +// +// Usage: +// KJ_TRY { +// someCode(); +// } KJ_CATCH (kj::Exception& exception) { +// handleException(exception); +// } +// +// This expands to a nested try-catch where the inner catch converts any exception +// to kj::Exception using getCaughtExceptionAsKj(), then re-throws it so the outer +// catch can handle it by specific type. +#define KJ_TRY \ + try { \ + try +#define KJ_CATCH \ + catch (...) { \ + throw kj::getCaughtExceptionAsKj(); \ + } \ + } \ + catch