Skip to content

Commit

Permalink
Use JsValue in AbortSignal implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell committed Aug 18, 2023
1 parent e9349d9 commit 570d259
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 31 deletions.
26 changes: 13 additions & 13 deletions src/workerd/api/basics.c++
Original file line number Diff line number Diff line change
Expand Up @@ -361,30 +361,30 @@ bool EventTarget::dispatchEvent(jsg::Lock& js, jsg::Ref<Event> event) {

kj::Exception AbortSignal::abortException(
jsg::Lock& js,
jsg::Optional<v8::Local<v8::Value>> maybeReason) {
jsg::Optional<jsg::JsValue> maybeReason) {
KJ_IF_MAYBE(reason, maybeReason) {
return js.exceptionToKj(js.v8Ref(*reason));
return js.exceptionToKj(*reason);
}

return JSG_KJ_EXCEPTION(DISCONNECTED, DOMAbortError, "The operation was aborted");
}

jsg::Ref<AbortSignal> AbortSignal::abort(
jsg::Lock& js,
jsg::Optional<v8::Local<v8::Value>> maybeReason) {
jsg::Optional<jsg::JsValue> maybeReason) {
auto exception = abortException(js, maybeReason);
KJ_IF_MAYBE(reason, maybeReason) {
return jsg::alloc<AbortSignal>(kj::mv(exception), js.v8Ref(*reason));
return jsg::alloc<AbortSignal>(kj::mv(exception), reason->addRef(js));
}
return jsg::alloc<AbortSignal>(kj::cp(exception), js.exceptionToJs(kj::mv(exception)));
return jsg::alloc<AbortSignal>(kj::cp(exception), js.exceptionToJsValue(kj::mv(exception)));
}

void AbortSignal::throwIfAborted(jsg::Lock& js) {
if (canceler->isCanceled()) {
KJ_IF_MAYBE(r, reason) {
js.throwException(r->addRef(js));
js.throwException(r->getHandle(js));
} else {
js.throwException(js.exceptionToJs(abortException(js)));
js.throwException(abortException(js));
}
}
}
Expand All @@ -402,7 +402,7 @@ jsg::Ref<AbortSignal> AbortSignal::timeout(jsg::Lock& js, double delay) {
// completes, whichever comes first.

global.setTimeoutInternal([signal = signal.addRef()](jsg::Lock& js) mutable {
auto exception = js.exceptionToJs(JSG_KJ_EXCEPTION(FAILED,
auto exception = js.exceptionToJsValue(JSG_KJ_EXCEPTION(FAILED,
DOMTimeoutError, "The operation was aborted due to timeout"));

signal->triggerAbort(js, exception.getHandle(js));
Expand Down Expand Up @@ -472,16 +472,16 @@ RefcountedCanceler& AbortSignal::getCanceler() {

void AbortSignal::triggerAbort(
jsg::Lock& js,
jsg::Optional<v8::Local<v8::Value>> maybeReason) {
jsg::Optional<jsg::JsValue> maybeReason) {
KJ_ASSERT(flag != Flag::NEVER_ABORTS);
if (canceler->isCanceled()) {
return;
}
auto exception = AbortSignal::abortException(js, maybeReason);
KJ_IF_MAYBE(r, maybeReason) {
reason = js.v8Ref(*r);
reason = r->addRef(js);
} else {
reason = js.exceptionToJs(kj::mv(exception));
reason = js.exceptionToJsValue(kj::mv(exception));
}
canceler->cancel(kj::cp(exception));

Expand All @@ -498,7 +498,7 @@ void AbortSignal::triggerAbort(

void AbortController::abort(
jsg::Lock& js,
jsg::Optional<v8::Local<v8::Value>> maybeReason) {
jsg::Optional<jsg::JsValue> maybeReason) {
signal->triggerAbort(js, maybeReason);
}

Expand All @@ -524,7 +524,7 @@ kj::Promise<void> Scheduler::wait(
KJ_IF_MAYBE(options, maybeOptions) {
KJ_IF_MAYBE(s, options->signal) {
if ((*s)->getAborted()) {
return js.exceptionToKj(js.v8Ref((*s)->getReason(js)));
return js.exceptionToKj((*s)->getReason(js));
}
}
}
Expand Down
23 changes: 10 additions & 13 deletions src/workerd/api/basics.h
Original file line number Diff line number Diff line change
Expand Up @@ -466,27 +466,24 @@ class AbortSignal final: public EventTarget {
enum class Flag { NONE, NEVER_ABORTS };

AbortSignal(kj::Maybe<kj::Exception> exception = nullptr,
jsg::Optional<jsg::V8Ref<v8::Value>> maybeReason = nullptr,
jsg::Optional<jsg::JsRef<jsg::JsValue>> maybeReason = nullptr,
Flag flag = Flag::NONE) :
canceler(IoContext::current().addObject(
kj::refcounted<RefcountedCanceler>(kj::cp(exception)))),
flag(flag) {
KJ_IF_MAYBE(r, maybeReason) {
reason = kj::mv(*r);
}
}
flag(flag),
reason(kj::mv(maybeReason)) {}

// The AbortSignal explicitly does not expose a constructor(). It is
// illegal for user code to create an AbortSignal directly.
static jsg::Ref<AbortSignal> constructor() = delete;

bool getAborted() { return canceler->isCanceled(); }

v8::Local<v8::Value> getReason(jsg::Lock& js) {
jsg::JsValue getReason(jsg::Lock& js) {
KJ_IF_MAYBE(r, reason) {
return r->getHandle(js);
}
return js.v8Undefined();
return js.undefined();
}

void throwIfAborted(jsg::Lock& js);
Expand All @@ -496,7 +493,7 @@ class AbortSignal final: public EventTarget {

static jsg::Ref<AbortSignal> abort(
jsg::Lock& js,
jsg::Optional<v8::Local<v8::Value>> reason);
jsg::Optional<jsg::JsValue> reason);
// The static abort() function here returns an AbortSignal that
// has been pre-emptively aborted. It's useful when it might still
// be desirable to kick off an async process while communicating
Expand All @@ -505,7 +502,7 @@ class AbortSignal final: public EventTarget {
static jsg::Ref<AbortSignal> timeout(jsg::Lock& js, double delay);
// Returns an AbortSignal that is triggered after delay milliseconds.

void triggerAbort(jsg::Lock& js, jsg::Optional<v8::Local<v8::Value>> maybeReason);
void triggerAbort(jsg::Lock& js, jsg::Optional<jsg::JsValue> maybeReason);

static jsg::Ref<AbortSignal> any(
jsg::Lock& js,
Expand Down Expand Up @@ -547,14 +544,14 @@ class AbortSignal final: public EventTarget {

static kj::Exception abortException(
jsg::Lock& js,
jsg::Optional<v8::Local<v8::Value>> reason = nullptr);
jsg::Optional<jsg::JsValue> reason = nullptr);

RefcountedCanceler& getCanceler();

private:
IoOwn<RefcountedCanceler> canceler;
Flag flag;
kj::Maybe<jsg::V8Ref<v8::Value>> reason;
kj::Maybe<jsg::JsRef<jsg::JsValue>> reason;

void visitForGc(jsg::GcVisitor& visitor);

Expand All @@ -572,7 +569,7 @@ class AbortController final: public jsg::Object {

jsg::Ref<AbortSignal> getSignal() { return signal.addRef(); }

void abort(jsg::Lock& js, jsg::Optional<v8::Local<v8::Value>> reason);
void abort(jsg::Lock& js, jsg::Optional<jsg::JsValue> reason);

JSG_RESOURCE_TYPE(AbortController, CompatibilityFlags::Reader flags) {
if (flags.getJsgPropertyOnPrototypeTemplate()) {
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/streams/internal.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,7 @@ bool WritableStreamInternalController::Pipe::checkSignal(jsg::Lock& js) {
auto reason = (*signal)->getReason(js);
if (!preventAbort) {
KJ_IF_MAYBE(writable, parent.state.tryGet<Writable>()) {
parent.state.get<Writable>()->abort(js.exceptionToKj(js.v8Ref(reason)));
parent.state.get<Writable>()->abort(js.exceptionToKj(reason));
parent.drain(js, reason);
} else {
parent.writeState.init<Unlocked>();
Expand All @@ -1488,7 +1488,7 @@ bool WritableStreamInternalController::Pipe::checkSignal(jsg::Lock& js) {
parent.writeState.init<Unlocked>();
}
if (!preventCancel) {
source.release(js, reason);
source.release(js, v8::Local<v8::Value>(reason));
} else {
source.release(js);
}
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/api/streams/standard.c++
Original file line number Diff line number Diff line change
Expand Up @@ -419,13 +419,13 @@ kj::Maybe<jsg::Promise<void>> WritableLockImpl<Controller>::PipeLocked::checkSig
if ((*signal)->getAborted()) {
auto reason = (*signal)->getReason(js);
if (!preventCancel) {
source.release(js, reason);
source.release(js, v8::Local<v8::Value>(reason));
} else {
source.release(js);
}
if (!preventAbort) {
return self.abort(js, reason).then(js,
JSG_VISITABLE_LAMBDA((this, reason = js.v8Ref(reason), ref = self.addRef()),
JSG_VISITABLE_LAMBDA((this, reason = reason.addRef(js), ref = self.addRef()),
(reason, ref), (jsg::Lock& js) {
return rejectedMaybeHandledPromise<void>(
js,
Expand Down Expand Up @@ -1071,7 +1071,7 @@ jsg::Promise<void> WritableImpl<Self>::abort(
jsg::Lock& js,
jsg::Ref<Self> self,
v8::Local<v8::Value> reason) {
signal->triggerAbort(js, reason);
signal->triggerAbort(js, jsg::JsValue(reason));

// We have to check this again after the AbortSignal is triggered.
if (state.template is<StreamStates::Closed>() || state.template is<StreamStates::Errored>()) {
Expand Down
6 changes: 6 additions & 0 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -1870,6 +1870,10 @@ class Lock {
// as an internal error: the KJ exception message is logged to stderr, and a JavaScript error
// is returned with a generic description.

JsRef<JsValue> exceptionToJsValue(kj::Exception&& exception);

kj::Exception exceptionToKj(const JsValue& exception);

kj::Exception exceptionToKj(Value&& exception);
// Encodes the given JavaScript exception into a KJ exception, formatting the description in
// such a way that hopefully exceptionToJs() can reproduce something equivalent to the original
Expand All @@ -1884,6 +1888,8 @@ class Lock {
throwException(exceptionToJs(kj::mv(exception)));
}

[[noreturn]] void throwException(const JsValue& exception);

template <typename Func, typename ErrorHandler>
auto tryCatch(Func&& func, ErrorHandler&& errorHandler) -> decltype(func()) {
// Invokes `func()` synchronously, catching exceptions. In the event of an exception,
Expand Down
20 changes: 20 additions & 0 deletions src/workerd/jsg/util.c++
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,27 @@ Value Lock::exceptionToJs(kj::Exception&& exception) {
});
}

JsRef<JsValue> Lock::exceptionToJsValue(kj::Exception&& exception) {
return withinHandleScope([&] {
JsValue val = JsValue(makeInternalError(v8Isolate, kj::mv(exception)));
return val.addRef(*this);
});
}

void Lock::throwException(Value&& exception) {
withinHandleScope([&] {
v8Isolate->ThrowException(exception.getHandle(*this));
});
throw JsExceptionThrown();
}

void Lock::throwException(const JsValue& exception) {
withinHandleScope([&] {
v8Isolate->ThrowException(exception);
});
throw JsExceptionThrown();
}

void throwInternalError(v8::Isolate* isolate, kj::StringPtr internalMessage) {
isolate->ThrowException(makeInternalError(isolate, internalMessage));
}
Expand Down Expand Up @@ -467,6 +481,12 @@ kj::Exception Lock::exceptionToKj(Value&& exception) {
});
}

kj::Exception Lock::exceptionToKj(const JsValue& exception) {
return withinHandleScope([&] {
return createTunneledException(v8Isolate, exception);
});
}

static kj::byte DUMMY = 0;
static kj::Array<kj::byte> getEmptyArray() {
// An older version of asBytes(), when given an empty ArrayBuffer, would often return an array
Expand Down

0 comments on commit 570d259

Please sign in to comment.