src: convey potential exceptions during StreamPipe construction

This moves the V8 calls during the StreamPipe construction to an overload of StreamPipe::New(), so that it is possible to indicate if there is a pending exception/termination. Refs: nodejs/node#40425 (comment) Signed-off-by: Darshan Sen <[email protected]> PR-URL: nodejs/node#43240 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
noslate-project · Oct 10, 2022 · fd40de1 · fd40de1
1 parent 2d53bb9
commit fd40de1
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 30 deletions.
diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc
@@ -11,7 +11,10 @@ using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
+using v8::Just;
 using v8::Local;
+using v8::Maybe;
+using v8::Nothing;
 using v8::Object;
 using v8::Value;
 
@@ -28,31 +31,6 @@ StreamPipe::StreamPipe(StreamBase* source,
   sink->PushStreamListener(&writable_listener_);
 
   uses_wants_write_ = sink->HasWantsWrite();
-
-  // Set up links between this object and the source/sink objects.
-  // In particular, this makes sure that they are garbage collected as a group,
-  // if that applies to the given streams (for example, Http2Streams use
-  // weak references).
-  if (obj->Set(env()->context(),
-               env()->source_string(),
-               source->GetObject()).IsNothing()) {
-    return;
-  }
-  if (source->GetObject()->Set(env()->context(),
-                               env()->pipe_target_string(),
-                               obj).IsNothing()) {
-      return;
-  }
-  if (obj->Set(env()->context(),
-               env()->sink_string(),
-               sink->GetObject()).IsNothing()) {
-    return;
-  }
-  if (sink->GetObject()->Set(env()->context(),
-                             env()->pipe_source_string(),
-                             obj).IsNothing()) {
-    return;
-  }
 }
 
 StreamPipe::~StreamPipe() {
@@ -261,14 +239,46 @@ void StreamPipe::WritableListener::OnStreamRead(ssize_t nread,
   return previous_listener_->OnStreamRead(nread, buf);
 }
 
+Maybe<StreamPipe*> StreamPipe::New(StreamBase* source,
+                                   StreamBase* sink,
+                                   Local<Object> obj) {
+  std::unique_ptr<StreamPipe> stream_pipe(new StreamPipe(source, sink, obj));
+
+  // Set up links between this object and the source/sink objects.
+  // In particular, this makes sure that they are garbage collected as a group,
+  // if that applies to the given streams (for example, Http2Streams use
+  // weak references).
+  Environment* env = source->stream_env();
+  if (obj->Set(env->context(), env->source_string(), source->GetObject())
+          .IsNothing()) {
+    return Nothing<StreamPipe*>();
+  }
+  if (source->GetObject()
+          ->Set(env->context(), env->pipe_target_string(), obj)
+          .IsNothing()) {
+    return Nothing<StreamPipe*>();
+  }
+  if (obj->Set(env->context(), env->sink_string(), sink->GetObject())
+          .IsNothing()) {
+    return Nothing<StreamPipe*>();
+  }
+  if (sink->GetObject()
+          ->Set(env->context(), env->pipe_source_string(), obj)
+          .IsNothing()) {
+    return Nothing<StreamPipe*>();
+  }
+
+  return Just(stream_pipe.release());
+}
+
 void StreamPipe::New(const FunctionCallbackInfo<Value>& args) {
   CHECK(args.IsConstructCall());
   CHECK(args[0]->IsObject());
   CHECK(args[1]->IsObject());
   StreamBase* source = StreamBase::FromObject(args[0].As<Object>());
   StreamBase* sink = StreamBase::FromObject(args[1].As<Object>());
 
-  new StreamPipe(source, sink, args.This());
+  if (StreamPipe::New(source, sink, args.This()).IsNothing()) return;
 }
 
 void StreamPipe::Start(const FunctionCallbackInfo<Value>& args) {

diff --git a/src/stream_pipe.h b/src/stream_pipe.h
@@ -13,10 +13,9 @@ class StreamPipe : public AsyncWrap {
 
   void Unpipe(bool is_in_deletion = false);
 
-  // TODO(RaisinTen): Just like MessagePort, add the following overload:
-  // static StreamPipe* New(StreamBase* source, StreamBase* sink,
-  //                        v8::Local<v8::Object> obj);
-  // so that we can indicate if there is a pending exception/termination.
+  static v8::Maybe<StreamPipe*> New(StreamBase* source,
+                                    StreamBase* sink,
+                                    v8::Local<v8::Object> obj);
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Unpipe(const v8::FunctionCallbackInfo<v8::Value>& args);