Draft: Cancellation of FSReq

src/api/environment.cc

@@ -517,6 +517,21 @@ MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
   return handle_scope.Escape(exports);
 }
 
+MaybeLocal<Function> GetDOMException(Local<Context> context) {
+  Isolate* isolate = context->GetIsolate();
+  Local<Object> per_context_bindings;
+  Local<Value> domexception_ctor_val;
+  if (!GetPerContextExports(context).ToLocal(&per_context_bindings) ||
+      !per_context_bindings
+           ->Get(context, FIXED_ONE_BYTE_STRING(isolate, "DOMException"))
+           .ToLocal(&domexception_ctor_val)) {
+    return MaybeLocal<Function>();
+  }
+  CHECK(domexception_ctor_val->IsFunction());
+  Local<Function> domexception_ctor = domexception_ctor_val.As<Function>();
+  return domexception_ctor;
+}
+
 // Any initialization logic should be performed in
 // InitializeContext, because embedders don't necessarily
 // call NewContext and so they will experience breakages.

src/node_file-inl.h

@@ -182,12 +182,17 @@ void FSReqPromise<AliasedBufferT>::Reject(v8::Local<v8::Value> reject) {
       object()->Get(env()->context(),
                     env()->promise_string()).ToLocalChecked();
   v8::Local<v8::Promise::Resolver> resolver = value.As<v8::Promise::Resolver>();
+  MaybeReplaceWithAbortError(&reject);
   USE(resolver->Reject(env()->context(), reject).FromJust());
 }
 
 template <typename AliasedBufferT>
 void FSReqPromise<AliasedBufferT>::Resolve(v8::Local<v8::Value> value) {
   finished_ = true;
+  if (IsCanceled()) {
+    Reject(v8::Undefined(env()->isolate()));
+    return;
+  }
   v8::HandleScope scope(env()->isolate());
   InternalCallbackScope callback_scope(this);
   v8::Local<v8::Value> val =

src/node_file.cc

@@ -130,6 +130,32 @@ FileHandleReadWrap::~FileHandleReadWrap() {}
 
 FSReqBase::~FSReqBase() {}
 
+void FSReqBase::Cancel(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  FSReqBase* self;
+  ASSIGN_OR_RETURN_UNWRAP(&self, args.Holder());
+  self->ReqWrap::Cancel();
+  self->is_canceled_ = true;
+}
+
+void FSReqBase::MaybeReplaceWithAbortError(v8::Local<v8::Value>* reject) {
+  if (!is_canceled_) return;
+
+  Local<Value> exception;
+  Local<Function> domexception_ctor;
+  if (!GetDOMException(env()->context()).ToLocal(&domexception_ctor)) return;
+
+  Local<Value> argv[] = {
+      FIXED_ONE_BYTE_STRING(env()->isolate(), "Operation was cancelled."),
+      FIXED_ONE_BYTE_STRING(env()->isolate(), "AbortError"),
+  };
+  Local<Value> abort_error;
+  if (!domexception_ctor->NewInstance(env()->context(), arraysize(argv), argv)
+           .ToLocal(&abort_error))
+    return;
+
+  *reject = abort_error;
+}
+
 void FSReqBase::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("continuation_data", continuation_data_);
 }
@@ -555,6 +581,7 @@ int FileHandle::DoShutdown(ShutdownWrap* req_wrap) {
 
 
 void FSReqCallback::Reject(Local<Value> reject) {
+  MaybeReplaceWithAbortError(&reject);
   MakeCallback(env()->oncomplete_string(), 1, &reject);
 }
 
@@ -563,6 +590,13 @@ void FSReqCallback::ResolveStat(const uv_stat_t* stat) {
 }
 
 void FSReqCallback::Resolve(Local<Value> value) {
+  // If FSReqBase::Cancel() was called, then reject the request rather than
+  // resolving.
+  if (IsCanceled()) {
+    Reject(Undefined(env()->isolate()));
+    return;
+  }
+
   Local<Value> argv[2] {
     Null(env()->isolate()),
     value
@@ -2462,6 +2496,7 @@ void Initialize(Local<Object> target,
   fst->InstanceTemplate()->SetInternalFieldCount(
       FSReqBase::kInternalFieldCount);
   fst->Inherit(AsyncWrap::GetConstructorTemplate(env));
+  env->SetProtoMethod(fst, "cancel", FSReqBase::Cancel);
   Local<String> wrapString =
       FIXED_ONE_BYTE_STRING(isolate, "FSReqCallback");
   fst->SetClassName(wrapString);
@@ -2485,6 +2520,7 @@ void Initialize(Local<Object> target,
   // Create Function Template for FSReqPromise
   Local<FunctionTemplate> fpt = FunctionTemplate::New(isolate);
   fpt->Inherit(AsyncWrap::GetConstructorTemplate(env));
+  env->SetProtoMethod(fpt, "cancel", FSReqBase::Cancel);
   Local<String> promiseString =
       FIXED_ONE_BYTE_STRING(isolate, "FSReqPromise");
   fpt->SetClassName(promiseString);

src/node_file.h

@@ -85,6 +85,9 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
   virtual void SetReturnValue(
       const v8::FunctionCallbackInfo<v8::Value>& args) = 0;
 
+  // JS-exposed method to cancel the UV request.
+  static void Cancel(const v8::FunctionCallbackInfo<v8::Value>& args);
+
   const char* syscall() const { return syscall_; }
   const char* data() const { return has_data_ ? *buffer_ : nullptr; }
   enum encoding encoding() const { return encoding_; }
@@ -111,12 +114,17 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
 
   BindingData* binding_data() { return binding_data_.get(); }
 
+ protected:
+  bool IsCanceled() { return is_canceled_; }
+  void MaybeReplaceWithAbortError(v8::Local<v8::Value>* value);
+
  private:
   std::unique_ptr<FSContinuationData> continuation_data_;
   enum encoding encoding_ = UTF8;
   bool has_data_ = false;
   bool use_bigint_ = false;
   bool is_plain_open_ = false;
+  bool is_canceled_ = false;
   const char* syscall_ = nullptr;
 
   BaseObjectPtr<BindingData> binding_data_;

src/node_internals.h

@@ -301,6 +301,7 @@ v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
 v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
                                          StartExecutionCallback cb = nullptr);
 v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
+v8::MaybeLocal<v8::Function> GetDOMException(v8::Local<v8::Context> context);
 v8::MaybeLocal<v8::Value> ExecuteBootstrapper(
     Environment* env,
     const char* id,

src/node_messaging.cc

@@ -225,21 +225,6 @@ MaybeLocal<Function> GetEmitMessageFunction(Local<Context> context) {
   return emit_message_val.As<Function>();
 }
 
-MaybeLocal<Function> GetDOMException(Local<Context> context) {
-  Isolate* isolate = context->GetIsolate();
-  Local<Object> per_context_bindings;
-  Local<Value> domexception_ctor_val;
-  if (!GetPerContextExports(context).ToLocal(&per_context_bindings) ||
-      !per_context_bindings->Get(context,
-                                FIXED_ONE_BYTE_STRING(isolate, "DOMException"))
-          .ToLocal(&domexception_ctor_val)) {
-    return MaybeLocal<Function>();
-  }
-  CHECK(domexception_ctor_val->IsFunction());
-  Local<Function> domexception_ctor = domexception_ctor_val.As<Function>();
-  return domexception_ctor;
-}
-
 void ThrowDataCloneException(Local<Context> context, Local<String> message) {
   Isolate* isolate = context->GetIsolate();
   Local<Value> argv[] = {message,

test/cctest/test_environment.cc

@@ -559,3 +559,22 @@ TEST_F(EnvironmentTest, SetImmediateMicrotasks) {
 
   EXPECT_EQ(called, 1);
 }
+
+TEST_F(EnvironmentTest, GetDOMExceptionTest) {
+  // Test that GetDOMException() returns a constructor named "DOMException"
+  const v8::HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env{handle_scope, argv};
+
+  v8::Local<v8::Function> domexception_ctor =
+      node::GetDOMException(env.context()).ToLocalChecked();
+
+  CHECK(domexception_ctor->IsConstructor());
+
+  char domexception[13];
+  v8::Local<v8::Value> v_name = domexception_ctor->GetName();
+  CHECK(v_name->IsString());
+  v8::Local<v8::String> name = v_name->ToString(env.context()).ToLocalChecked();
+  name->WriteUtf8(isolate_, domexception, 13);
+  EXPECT_STREQ(domexception, "DOMException");
+}

test/parallel/test-fsreqcallback-cancel.js

@@ -0,0 +1,65 @@
+'use strict';
+// Flags: --no-warnings --expose-internals
+
+const common = require('../common');
+const assert = require('assert');
+const { internalBinding } = require('internal/test/binding');
+const path = require('path');
+
+const { FSReqCallback, readdir } = internalBinding('fs');
+
+{
+  // Return value of cancel() is undefined
+  const req = new FSReqCallback();
+  req.oncomplete = () => {};
+  assert.strictEqual(req.cancel(), undefined);
+}
+
+
+{
+  // Callback is called with AbortError if request is canceled
+  const req = new FSReqCallback();
+  req.oncomplete = common.mustCall((err, files) => {
+    assert.strictEqual(files, undefined);
+    assert.strictEqual(err.name, 'AbortError');
+  }, 1);
+  req.cancel();
+  readdir(path.toNamespacedPath('../'), 'utf8', false, req);
+}
+
+
+{
+  // Request is canceled if cancel() called before control returns to main loop
+  const req = new FSReqCallback();
+  req.oncomplete = common.mustCall((err, files) => {
+    assert.strictEqual(files, undefined);
+    assert.strictEqual(err.name, 'AbortError');
+  }, 1);
+  readdir(path.toNamespacedPath('../'), 'utf8', false, req);
+  req.cancel();
+}
+
+
+{
+  // Request is still canceled on next tick
+  const req = new FSReqCallback();
+  req.oncomplete = common.mustCall((err, files) => {
+    assert.strictEqual(files, undefined);
+    assert.strictEqual(err.name, 'AbortError');
+  }, 1);
+  readdir(path.toNamespacedPath('../'), 'utf8', false, req);
+  process.nextTick(common.mustCall(() => req.cancel()));
+}
+
+
+{
+  // Callback is not called a second time if request canceled after it has
+  // already completed
+  const req = new FSReqCallback();
+  req.oncomplete = common.mustCall((err, files) => {
+    assert.strictEqual(err, null);
+    assert.notStrictEqual(files, undefined);
+    req.cancel();
+  }, 1);
+  readdir(path.toNamespacedPath('../'), 'utf8', false, req);
+}