From 795e7d77e6f381e10a477189b22c9aa8644c3a32 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 21 Jan 2025 14:11:02 -0800 Subject: [PATCH] Remove obsolete info warnings for cross request promises (#3388) --- src/workerd/api/basics.c++ | 10 ---------- src/workerd/api/blob.c++ | 6 ------ src/workerd/api/tests/abortsignal-test.js | 6 +----- src/workerd/io/worker.c++ | 24 ----------------------- 4 files changed, 1 insertion(+), 45 deletions(-) diff --git a/src/workerd/api/basics.c++ b/src/workerd/api/basics.c++ index b43f76dde28..8eb290d17f7 100644 --- a/src/workerd/api/basics.c++ +++ b/src/workerd/api/basics.c++ @@ -508,16 +508,6 @@ void AbortSignal::setOnAbort(jsg::Lock& js, jsg::Optional handler) if (h.isFunction() || h.isObject()) { onAbortHandler = jsg::JsRef(js, h); return; - } else { - // TODO(soon): Per the spec we are supposed o set the handler to null if it is not - // a function or an object. However, there's an ever so slight change that would - // be breaking. So let's go ahead and set the value in this case and log a warning. - // If we do not see any instances of the warning in logs, we can remove this and - // go with the default behavior. - LOG_WARNING_PERIODICALLY( - "NOSENTRY AbortSignal::setOnAbort set to non-function/non-object value"); - onAbortHandler = jsg::JsRef(js, h); - return; } } onAbortHandler = kj::none; diff --git a/src/workerd/api/blob.c++ b/src/workerd/api/blob.c++ index ac61429bdfd..9503091a70e 100644 --- a/src/workerd/api/blob.c++ +++ b/src/workerd/api/blob.c++ @@ -52,12 +52,6 @@ kj::Array concat(jsg::Lock& js, jsg::Optional maybeBits) { partSize <= upperLimit, RangeError, kj::str("Blob part too large: ", partSize, " bytes")); // Checks for oversize - if (size + partSize > maxBlobSize) { - // TODO(soon): This logging is just to help us determine further how common - // this case is. We can and should remove the logging once we have enough data. - LOG_WARNING_PERIODICALLY( - kj::str("NOSENTRY Attempt to create a Blob with size ", size + partSize)); - } JSG_REQUIRE(size + partSize <= maxBlobSize, RangeError, kj::str("Blob size ", size + partSize, " exceeds limit ", maxBlobSize)); size += partSize; diff --git a/src/workerd/api/tests/abortsignal-test.js b/src/workerd/api/tests/abortsignal-test.js index 3827e9cd472..270229dc1d7 100644 --- a/src/workerd/api/tests/abortsignal-test.js +++ b/src/workerd/api/tests/abortsignal-test.js @@ -180,11 +180,7 @@ export const onabortPrototypeProperty = { [123, null, 'foo'].forEach((v) => { ac.signal.onabort = () => {}; ac.signal.onabort = v; - // TODO(soon): For now, we are relaxing this check and will log a warning - // if the value is not a function or object. If we get no hits on that warning, - // we can return to checking for null here. - //strictEqual(ac.signal.onabort, null); - strictEqual(ac.signal.onabort, v); + strictEqual(ac.signal.onabort, null); }); const handler = {}; diff --git a/src/workerd/io/worker.c++ b/src/workerd/io/worker.c++ index 114ee240987..37e746ac256 100644 --- a/src/workerd/io/worker.c++ +++ b/src/workerd/io/worker.c++ @@ -1185,30 +1185,6 @@ Worker::Isolate::Isolate(kj::Own apiParam, try { auto& js = jsg::Lock::from(isolate); - { - // TODO(soon): Hopefully this logging is temporary. We want to get an idea - // of where the cross request promises are being resolved just in general. - // To do so we need to try to capture a stack. We do so by creating an error - // object and logging it. - v8::HandleScope handleScope(isolate); - auto err = - v8::Exception::Error(js.str("Cross Request Promise Resolve"_kj)).As(); - jsg::check(err->Set(js.v8Context(), js.str("name"_kj), js.str("Warning"_kj))); - auto stack = jsg::check(err->Get(js.v8Context(), js.str("stack"_kj))); - auto msg = kj::str("NOSENTRY ", stack); - if (msg != "NOSENTRY Warning: Cross Request Promise Resolve") { - LOG_PERIODICALLY(WARNING, msg); - } else { - // If we get here, it means we don't have a useful JS stack trace. - // Either the stack trace limit was set to zero for some reason or - // this resolve/reject originated from inside the runtime where a - // JS stack trace is not available. Let's emit the warning with a - // C++ stack instead. - // TODO(review): Is this worthwhile? Does it give us enough useful signal? - LOG_PERIODICALLY(WARNING, kj::str(msg, "\n", kj::getStackTrace())); - } - } - // The promise tag is generally opaque except for right here. The tag // wraps an instanceof kj::Own, which wraps an atomically // refcounted pointer to the DeleteQueue for the correct isolate.