Skip to content

Commit

Permalink
Add workerd-autogate-internal-error-id autogate
Browse files Browse the repository at this point in the history
...and only render internal error reference IDs in user-facing "internal error"
exceptions if autogate is enabled.
  • Loading branch information
jclee committed Feb 7, 2025
1 parent 9c7db5f commit 51123f6
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 8 deletions.
2 changes: 2 additions & 0 deletions src/workerd/jsg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ wd_cc_library(
":observer",
":url",
"//src/workerd/util",
"//src/workerd/util:autogate",
"//src/workerd/util:sentry",
"//src/workerd/util:thread-scopes",
"//src/workerd/util:uuid",
Expand Down Expand Up @@ -177,6 +178,7 @@ wd_cc_library(
src = f,
deps = [
":jsg",
"//src/workerd/util:autogate",
],
) for f in glob(
["*-test.c++"],
Expand Down
104 changes: 102 additions & 2 deletions src/workerd/jsg/util-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "jsg-test.h"
#include "jsg.h"

#include <workerd/util/autogate.h>

namespace workerd::jsg::test {
namespace {

Expand Down Expand Up @@ -112,8 +114,42 @@ struct ThrowContext: public ContextGlobalObject {
};
JSG_DECLARE_ISOLATE_TYPE(ThrowIsolate, ThrowContext);

KJ_TEST("throw internal error") {
KJ_TEST("throw internal error without internal-error-id autogate") {
setPredictableModeForTest();
util::Autogate::initAutogateNamesForTest({}); // without internal-error-id autogate
KJ_DEFER(util::Autogate::deinitAutogate());

Evaluator<ThrowContext, ThrowIsolate> e(v8System);
{
KJ_EXPECT_LOG(ERROR, "thrown from throwException");
e.expectEval("throwException()", "throws", "Error: internal error");
}
{
// We also expect the logged internal error to contain the error id.
KJ_EXPECT_LOG(ERROR, "wdErrId = 0123456789abcdefghijklmn");
e.expectEval("throwException()", "throws", "Error: internal error");
}

{
KJ_EXPECT_LOG(ERROR, "thrown from getThrowing");
e.expectEval("throwing", "throws", "Error: internal error");
}

{
KJ_EXPECT_LOG(ERROR, "thrown from setThrowing");
e.expectEval("throwing = 123", "throws", "Error: internal error");
}

{
KJ_EXPECT_LOG(ERROR, "thrown from returnFunctionThatThrows");
e.expectEval("returnFunctionThatThrows(123)(321)", "throws", "Error: internal error");
}
}

KJ_TEST("throw internal error with internal-error-id autogate") {
setPredictableModeForTest();
util::Autogate::initAutogateNamesForTest({"internal-error-id"_kj});
KJ_DEFER(util::Autogate::deinitAutogate());

Evaluator<ThrowContext, ThrowIsolate> e(v8System);
{
Expand Down Expand Up @@ -273,8 +309,72 @@ struct TunneledContext: public ContextGlobalObject {
};
JSG_DECLARE_ISOLATE_TYPE(TunneledIsolate, TunneledContext);

KJ_TEST("throw tunneled exception") {
KJ_TEST("throw tunneled exception without internal-error-id autogate") {
setPredictableModeForTest();
util::Autogate::initAutogateNamesForTest({}); // without internal-error-id autogate
KJ_DEFER(util::Autogate::deinitAutogate());

Evaluator<TunneledContext, TunneledIsolate> e(v8System);
e.expectEval(
"throwTunneledTypeError()", "throws", "TypeError: thrown from throwTunneledTypeError");
e.expectEval("throwTunneledTypeErrorLateColon()", "throws", "TypeError");
e.expectEval("throwTunneledTypeErrorWithExpectation()", "throws",
"TypeError: thrown from throwTunneledTypeErrorWithExpectation");
e.expectEval("throwTunneledOperationError()", "throws",
"OperationError: thrown from throwTunneledOperationError");
e.expectEval("throwTunneledOperationErrorWithoutMessage()", "throws", "OperationError");
e.expectEval("throwTunneledOperationErrorLateColon()", "throws", "OperationError");
e.expectEval("throwTunneledOperationErrorWithExpectation()", "throws",
"OperationError: thrown from throwTunneledOperationErrorWithExpectation");
{
KJ_EXPECT_LOG(ERROR, "thrown from throwTunneledInternalOperationError");
e.expectEval(
"throwTunneledInternalOperationError()", "throws", "OperationError: internal error");
}
{
// We also expect the logged internal error to contain the error id.
KJ_EXPECT_LOG(ERROR, "wdErrId = 0123456789abcdefghijklmn");
e.expectEval(
"throwTunneledInternalOperationError()", "throws", "OperationError: internal error");
}
{
KJ_EXPECT_LOG(ERROR, " jsg.TypeError");
e.expectEval("throwBadTunneledError()", "throws", "Error: internal error");
}
{
KJ_EXPECT_LOG(ERROR, "expected s.startsWith(\";\"); jsg.TypeError");
e.expectEval("throwBadTunneledErrorWithExpectation()", "throws", "Error: internal error");
}
e.expectEval("throwTunneledMacroTypeError()", "throws",
"TypeError: thrown from throwTunneledMacroTypeError");
e.expectEval("throwTunneledMacroTypeErrorWithExpectation()", "throws",
"TypeError: thrown from throwTunneledMacroTypeErrorWithExpectation");
e.expectEval("throwTunneledMacroOperationError()", "throws",
"OperationError: thrown from throwTunneledMacroOperationError");
e.expectEval("throwTunneledMacroOperationErrorWithExpectation()", "throws",
"OperationError: thrown from throwTunneledMacroOperationErrorWithExpectation");
e.expectEval("throwTunneledCompileError()", "throws",
"CompileError: thrown from throwTunneledCompileError");
e.expectEval(
"throwTunneledLinkError()", "throws", "CompileError: thrown from throwTunneledLinkError");
e.expectEval("throwTunneledRuntimeError()", "throws",
"CompileError: thrown from throwTunneledRuntimeError");
e.expectEval(
"throwTunneledDOMException()", "throws", "Some error: thrown from throwTunneledDOMException");
{
KJ_EXPECT_LOG(ERROR, " thrown from throwTunneledInvalidDOMException");
e.expectEval("throwTunneledInvalidDOMException()", "throws", "Error: internal error");
}
{
KJ_EXPECT_LOG(ERROR, " thrown from throwTunneledGarbledDOMException");
e.expectEval("throwTunneledGarbledDOMException()", "throws", "Error: internal error");
}
}

KJ_TEST("throw tunneled exception with internal-error-id autogate") {
setPredictableModeForTest();
util::Autogate::initAutogateNamesForTest({"internal-error-id"_kj});
KJ_DEFER(util::Autogate::deinitAutogate());

Evaluator<TunneledContext, TunneledIsolate> e(v8System);
e.expectEval(
Expand Down
12 changes: 6 additions & 6 deletions src/workerd/jsg/util.c++
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <cxxabi.h>
#endif

#include <workerd/util/autogate.h>
#include <workerd/util/sentry.h>

namespace workerd::jsg {
Expand Down Expand Up @@ -125,12 +126,11 @@ InternalErrorId makeInternalErrorId() {
}

kj::String renderInternalError(InternalErrorId& internalErrorId) {
// TODO(now): put "internal error" change behind autogate or compatibility flag?
//
// It's possible that existing user error handling systems could rely on an exact match with the
// existing "internal error" string. On the other hand, feature flags need a jsg::Lock to read,
// which may not be available in all contexts that generate internal errors?
return kj::str("internal error; reference = ", internalErrorId);
if (util::Autogate::isEnabled(util::AutogateKey::INTERNAL_ERROR_ID)) {
return kj::str("internal error; reference = ", internalErrorId);
} else {
return kj::str("internal error");
}
}

} // namespace
Expand Down
2 changes: 2 additions & 0 deletions src/workerd/util/autogate.c++
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ kj::StringPtr KJ_STRINGIFY(AutogateKey key) {
return "streaming-tail-workers"_kj;
case AutogateKey::PYTHON_FETCH_INDIVIDUAL_PACKAGES:
return "python-fetch-individual-packages";
case AutogateKey::INTERNAL_ERROR_ID:
return "internal-error-id";
case AutogateKey::NumOfKeys:
KJ_FAIL_ASSERT("NumOfKeys should not be used in getName");
}
Expand Down
2 changes: 2 additions & 0 deletions src/workerd/util/autogate.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ enum class AutogateKey {
// Fetches Python packages as individual bundles from GCS instead of using a single big bundle
// embedded in the binary
PYTHON_FETCH_INDIVIDUAL_PACKAGES,
// Adds a "reference" ID to "internal error" exception messages
INTERNAL_ERROR_ID,
NumOfKeys // Reserved for iteration.
};

Expand Down

0 comments on commit 51123f6

Please sign in to comment.