Add macros to throw in debug builds and log in production

src/workerd/jsg/jsg.c++

@@ -77,13 +77,6 @@ void Data::destroy() {
   }
 }
 
-#ifdef KJ_DEBUG
-void Data::assertInvariantImpl() {
-    // Assert that only empty values are associated with null isolates.
-  KJ_DASSERT(isolate != nullptr || handle.IsEmpty());
-}
-#endif
-
 Lock::Lock(v8::Isolate* v8Isolate)
     : v8Isolate(v8Isolate), locker(v8Isolate), scope(v8Isolate),
       previousData(v8Isolate->GetData(2)),

src/workerd/jsg/jsg.h

@@ -11,6 +11,7 @@
 #include <kj/function.h>
 #include <kj/exception.h>
 #include <kj/one-of.h>
+#include <kj/debug.h>
 #include <type_traits>
 #include <v8.h>
 #include "macro-meta.h"
@@ -784,13 +785,13 @@ class Data {
 
   // Debugging helpers.
   void assertInvariant() {
-#ifdef KJ_DEBUG
-    assertInvariantImpl();
-#endif
+    // Assert that only empty values are associated with null isolates.
+    //
+    // Note that we use IASSERT (which is only enabled in debug) here because this function is
+    // intended to be invoked from the move ctor and assignment operator. We expect them to be
+    // invoked a lot and want them to be as optimizable as possible.
+    KJ_IASSERT(isolate != nullptr || handle.IsEmpty());
   }
-#ifdef KJ_DEBUG
-  void assertInvariantImpl();
-#endif
 };
 
 template <typename T>

src/workerd/jsg/setup.c++

@@ -391,14 +391,6 @@ bool IsolateBase::allowWasmCallback(
   return self->evalAllowed;
 }
 
-#ifdef KJ_DEBUG
-#define DEBUG_FAIL_PROD_LOG(...) \
-  KJ_FAIL_ASSERT(__VA_ARGS__);
-#else
-#define DEBUG_FAIL_PROD_LOG(...) \
-  KJ_LOG(ERROR, __VA_ARGS__);
-#endif
-
 void IsolateBase::jitCodeEvent(const v8::JitCodeEvent* event) noexcept {
   // We register this callback with V8 in order to build a mapping of code addresses to source
   // code locations, which we use when reporting stack traces during crashes.
@@ -462,7 +454,7 @@ void IsolateBase::jitCodeEvent(const v8::JitCodeEvent* event) noexcept {
 
     case v8::JitCodeEvent::CODE_REMOVED:
       if (!codeMap.erase(startAddr)) {
-        DEBUG_FAIL_PROD_LOG("CODE_REMOVED for unknown code block?");
+        DEBUG_FATAL_RELEASE_LOG(ERROR, "CODE_REMOVED for unknown code block?");
       }
       break;
 

src/workerd/util/sentry.h

@@ -86,4 +86,23 @@ inline kj::StringPtr maybeOmitColoFromSentry(uint32_t coloId) {
     }                                                                          \
   } while (0)
 
+// The DEBUG_FATAL_RELEASE_LOG macros is for assertions that should definitely break in tests but
+// are not worth breaking production over. Instead, it logs the assertion message to sentry so that
+// we can notice the event. If your code requires that an assertion is true for safety (e.g.
+// checking if a value is not null), this is not the macro for you.
+#ifdef KJ_DEBUG
+#define DEBUG_FATAL_RELEASE_LOG(severity, ...)                                 \
+  ([&]() noexcept {                                                            \
+    KJ_FAIL_ASSERT(__VA_ARGS__);                                               \
+  })()
+#else
+#define DEBUG_FATAL_RELEASE_LOG(severity, ...)                                 \
+  do {                                                                         \
+    static bool logOnce KJ_UNUSED = [&]() {                                    \
+      KJ_LOG(severity, __VA_ARGS__);                                           \
+      return true;                                                             \
+    }();                                                                       \
+  } while (0)
+#endif
+
 } // namespace workerd