Skip to content

Commit

Permalink
Remove obsolete info warnings for cross request promises (#3388)
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell authored Jan 21, 2025
1 parent 19b9a9b commit 795e7d7
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 45 deletions.
10 changes: 0 additions & 10 deletions src/workerd/api/basics.c++
Original file line number Diff line number Diff line change
Expand Up @@ -508,16 +508,6 @@ void AbortSignal::setOnAbort(jsg::Lock& js, jsg::Optional<jsg::JsValue> 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;
Expand Down
6 changes: 0 additions & 6 deletions src/workerd/api/blob.c++
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ kj::Array<byte> concat(jsg::Lock& js, jsg::Optional<Blob::Bits> 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;
Expand Down
6 changes: 1 addition & 5 deletions src/workerd/api/tests/abortsignal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down
24 changes: 0 additions & 24 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1185,30 +1185,6 @@ Worker::Isolate::Isolate(kj::Own<Api> 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<v8::Object>();
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<IoCrossContextExecutor>, which wraps an atomically
// refcounted pointer to the DeleteQueue for the correct isolate.
Expand Down

0 comments on commit 795e7d7

Please # to comment.