src: improve error handling in node_blob

PR-URL: #57078
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>

Author: jasnell

src/node_blob.cc

@@ -30,8 +30,10 @@ using v8::HandleScope;
 using v8::Int32;
 using v8::Isolate;
 using v8::Local;
+using v8::NewStringType;
 using v8::Object;
 using v8::ObjectTemplate;
+using v8::SnapshotCreator;
 using v8::String;
 using v8::Uint32;
 using v8::Undefined;
@@ -58,7 +60,7 @@ void Concat(const FunctionCallbackInfo<Value>& args) {
   std::vector<View> views;
   size_t total = 0;
 
-  std::vector<v8::Global<Value>> buffers;
+  std::vector<Global<Value>> buffers;
   if (FromV8Array(context, array, &buffers).IsNothing()) {
     return;
   }
@@ -108,17 +110,14 @@ void BlobFromFilePath(const FunctionCallbackInfo<Value>& args) {
   std::vector<std::unique_ptr<DataQueue::Entry>> entries;
   entries.push_back(std::move(entry));
 
-  auto blob =
-      Blob::Create(env, DataQueue::CreateIdempotent(std::move(entries)));
-
-  if (blob) {
-    auto array = Array::New(env->isolate(), 2);
-    USE(array->Set(env->context(), 0, blob->object()));
-    USE(array->Set(env->context(),
-                   1,
-                   Uint32::NewFromUnsigned(env->isolate(), blob->length())));
-
-    args.GetReturnValue().Set(array);
+  if (auto blob =
+          Blob::Create(env, DataQueue::CreateIdempotent(std::move(entries)))) {
+    Local<Value> vals[2]{
+        blob->object(),
+        Uint32::NewFromUnsigned(env->isolate(), blob->length()),
+    };
+    args.GetReturnValue().Set(
+        Array::New(env->isolate(), &vals[0], arraysize(vals)));
   }
 }
 }  // namespace
@@ -159,7 +158,7 @@ Local<FunctionTemplate> Blob::GetConstructorTemplate(Environment* env) {
   return tmpl;
 }
 
-bool Blob::HasInstance(Environment* env, v8::Local<v8::Value> object) {
+bool Blob::HasInstance(Environment* env, Local<Value> object) {
   return GetConstructorTemplate(env)->HasInstance(object);
 }
 
@@ -188,7 +187,7 @@ void Blob::New(const FunctionCallbackInfo<Value>& args) {
   Local<Array> array = args[0].As<Array>();
   std::vector<std::unique_ptr<DataQueue::Entry>> entries(array->Length());
 
-  std::vector<v8::Global<Value>> sources;
+  std::vector<Global<Value>> sources;
   if (FromV8Array(context, array, &sources).IsNothing()) {
     return;
   }
@@ -197,12 +196,16 @@ void Blob::New(const FunctionCallbackInfo<Value>& args) {
   for (size_t i = 0; i < count; i++) {
     Local<Value> entry = sources[i].Get(isolate);
 
-    const auto entryFromArrayBuffer = [isolate](v8::Local<v8::ArrayBuffer> buf,
-                                                size_t byte_length,
-                                                size_t byte_offset = 0) {
+    auto entryFromArrayBuffer =
+        [isolate](Local<ArrayBuffer> buf,
+                  size_t byte_length,
+                  size_t byte_offset =
+                      0) mutable -> std::unique_ptr<DataQueue::Entry> {
       if (buf->IsDetachable()) {
         std::shared_ptr<BackingStore> store = buf->GetBackingStore();
-        USE(buf->Detach(Local<Value>()));
+        if (buf->Detach(Local<Value>()).IsNothing()) {
+          return nullptr;
+        }
         return DataQueue::CreateInMemoryEntryFromBackingStore(
             store, byte_offset, byte_length);
       }
@@ -227,11 +230,15 @@ void Blob::New(const FunctionCallbackInfo<Value>& args) {
     // ensuring appropriate spec compliance.
     if (entry->IsArrayBuffer()) {
       Local<ArrayBuffer> buf = entry.As<ArrayBuffer>();
-      entries[i] = entryFromArrayBuffer(buf, buf->ByteLength());
+      auto ret = entryFromArrayBuffer(buf, buf->ByteLength());
+      if (!ret) return;
+      entries[i] = std::move(ret);
     } else if (entry->IsArrayBufferView()) {
       Local<ArrayBufferView> view = entry.As<ArrayBufferView>();
-      entries[i] = entryFromArrayBuffer(
+      auto ret = entryFromArrayBuffer(
           view->Buffer(), view->ByteLength(), view->ByteOffset());
+      if (!ret) return;
+      entries[i] = std::move(ret);
     } else if (Blob::HasInstance(env, entry)) {
       Blob* blob;
       ASSIGN_OR_RETURN_UNWRAP(&blob, entry);
@@ -279,22 +286,22 @@ BaseObjectPtr<Blob> Blob::Slice(Environment* env, size_t start, size_t end) {
 }
 
 Blob::Blob(Environment* env,
-           v8::Local<v8::Object> obj,
+           Local<Object> obj,
            std::shared_ptr<DataQueue> data_queue)
     : BaseObject(env, obj), data_queue_(data_queue) {
   MakeWeak();
 }
 
 Blob::Reader::Reader(Environment* env,
-                     v8::Local<v8::Object> obj,
+                     Local<Object> obj,
                      BaseObjectPtr<Blob> strong_ptr)
     : AsyncWrap(env, obj, AsyncWrap::PROVIDER_BLOBREADER),
       inner_(strong_ptr->data_queue_->get_reader()),
       strong_ptr_(std::move(strong_ptr)) {
   MakeWeak();
 }
 
-bool Blob::Reader::HasInstance(Environment* env, v8::Local<v8::Value> value) {
+bool Blob::Reader::HasInstance(Environment* env, Local<Value> value) {
   return GetConstructorTemplate(env)->HasInstance(value);
 }
 
@@ -370,7 +377,7 @@ void Blob::Reader::Pull(const FunctionCallbackInfo<Value>& args) {
       for (size_t n = 0; n < count; n++) total += vecs[n].len;
 
       std::shared_ptr<BackingStore> store =
-          v8::ArrayBuffer::NewBackingStore(env->isolate(), total);
+          ArrayBuffer::NewBackingStore(env->isolate(), total);
       auto ptr = static_cast<uint8_t*>(store->Data());
       for (size_t n = 0; n < count; n++) {
         std::copy(vecs[n].base, vecs[n].base + vecs[n].len, ptr);
@@ -415,7 +422,7 @@ std::unique_ptr<worker::TransferData> Blob::CloneForMessaging() const {
   return std::make_unique<BlobTransferData>(data_queue_);
 }
 
-void Blob::StoreDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
+void Blob::StoreDataObject(const FunctionCallbackInfo<Value>& args) {
   Realm* realm = Realm::GetCurrent(args);
 
   CHECK(args[0]->IsString());  // ID key
@@ -468,7 +475,7 @@ void Blob::RevokeObjectURL(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-void Blob::GetDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
+void Blob::GetDataObject(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsString());
   Realm* realm = Realm::GetCurrent(args);
   BlobBindingData* binding_data = realm->GetBindingData<BlobBindingData>();
@@ -482,7 +489,7 @@ void Blob::GetDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
     Local<Value> type;
     if (!String::NewFromUtf8(isolate,
                              stored.type.c_str(),
-                             v8::NewStringType::kNormal,
+                             NewStringType::kNormal,
                              static_cast<int>(stored.type.length()))
              .ToLocal(&type)) {
       return;
@@ -554,7 +561,7 @@ void BlobBindingData::Deserialize(Local<Context> context,
 }
 
 bool BlobBindingData::PrepareForSerialization(Local<Context> context,
-                                              v8::SnapshotCreator* creator) {
+                                              SnapshotCreator* creator) {
   // Stored blob objects are not actually persisted.
   // Return true because we need to maintain the reference to the binding from
   // JS land.