src: add error formatting support

PR-URL: nodejs#37598 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
mhdawson · Mar 7, 2022 · 22c5e46 · 22c5e46
1 parent 1e2257e
commit 22c5e46
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 29 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -2323,6 +2323,11 @@ than the parent module. Linked modules must share the same context.
 
 The linker function returned a module for which linking has failed.
 
+<a id="ERR_VM_MODULE_LINK_FAILURE"></a>
+### `ERR_VM_MODULE_LINK_FAILURE`
+
+The module was unable to be linked due to a failure.
+
 <a id="ERR_VM_MODULE_NOT_MODULE"></a>
 ### `ERR_VM_MODULE_NOT_MODULE`
 

diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js
@@ -317,6 +317,8 @@ class SourceTextModule extends Module {
           throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
         }
         if (module.status === 'errored') {
+          // TODO(devsnek): replace with ERR_VM_MODULE_LINK_FAILURE
+          // and error cause proposal.
           throw new ERR_VM_MODULE_LINKING_ERRORED();
         }
         if (module.status === 'unlinked') {

diff --git a/src/module_wrap.cc b/src/module_wrap.cc
@@ -297,7 +297,9 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
     Local<Value> resolve_return_value =
         maybe_resolve_return_value.ToLocalChecked();
     if (!resolve_return_value->IsPromise()) {
-      env->ThrowError("linking error, expected resolver to return a promise");
+      THROW_ERR_VM_MODULE_LINK_FAILURE(
+          env, "request for '%s' did not return promise", specifier_std);
+      return;
     }
     Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
     obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);
@@ -497,33 +499,35 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
 
   Isolate* isolate = env->isolate();
 
+  Utf8Value specifier_utf8(isolate, specifier);
+  std::string specifier_std(*specifier_utf8, specifier_utf8.length());
+
   ModuleWrap* dependent = GetFromModule(env, referrer);
   if (dependent == nullptr) {
-    env->ThrowError("linking error, null dep");
+    THROW_ERR_VM_MODULE_LINK_FAILURE(
+        env, "request for '%s' is from invalid module", specifier_std);
     return MaybeLocal<Module>();
   }
 
-  Utf8Value specifier_utf8(isolate, specifier);
-  std::string specifier_std(*specifier_utf8, specifier_utf8.length());
-
   if (dependent->resolve_cache_.count(specifier_std) != 1) {
-    env->ThrowError("linking error, not in local cache");
+    THROW_ERR_VM_MODULE_LINK_FAILURE(
+        env, "request for '%s' is not in cache", specifier_std);
     return MaybeLocal<Module>();
   }
 
   Local<Promise> resolve_promise =
       dependent->resolve_cache_[specifier_std].Get(isolate);
 
   if (resolve_promise->State() != Promise::kFulfilled) {
-    env->ThrowError("linking error, dependency promises must be resolved on "
-                    "instantiate");
+    THROW_ERR_VM_MODULE_LINK_FAILURE(
+        env, "request for '%s' is not yet fulfilled", specifier_std);
     return MaybeLocal<Module>();
   }
 
   Local<Object> module_object = resolve_promise->Result().As<Object>();
   if (module_object.IsEmpty() || !module_object->IsObject()) {
-    env->ThrowError("linking error, expected a valid module object from "
-                    "resolver");
+    THROW_ERR_VM_MODULE_LINK_FAILURE(
+        env, "request for '%s' did not return an object", specifier_std);
     return MaybeLocal<Module>();
   }
 

diff --git a/src/node_errors.h b/src/node_errors.h
@@ -3,6 +3,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include "debug_utils-inl.h"
 #include "env.h"
 #include "v8.h"
 
@@ -75,29 +76,40 @@ void OnFatalError(const char* location, const char* message);
   V(ERR_TLS_INVALID_PROTOCOL_METHOD, TypeError)                                \
   V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, Error)                                \
   V(ERR_VM_MODULE_CACHED_DATA_REJECTED, Error)                                 \
+  V(ERR_VM_MODULE_LINK_FAILURE, Error)                                         \
   V(ERR_WASI_NOT_STARTED, Error)                                               \
   V(ERR_WORKER_INIT_FAILED, Error)                                             \
-  V(ERR_PROTO_ACCESS, Error)                                                   \
+  V(ERR_PROTO_ACCESS, Error)
 
-#define V(code, type)                                                         \
-  inline v8::Local<v8::Value> code(v8::Isolate* isolate,                      \
-                                   const char* message)       {               \
-    v8::Local<v8::String> js_code = OneByteString(isolate, #code);            \
-    v8::Local<v8::String> js_msg = OneByteString(isolate, message);           \
-    v8::Local<v8::Object> e =                                                 \
-        v8::Exception::type(js_msg)->ToObject(                                \
-            isolate->GetCurrentContext()).ToLocalChecked();                   \
-    e->Set(isolate->GetCurrentContext(), OneByteString(isolate, "code"),      \
-           js_code).Check();                                                  \
-    return e;                                                                 \
-  }                                                                           \
-  inline void THROW_ ## code(v8::Isolate* isolate, const char* message) {     \
-    isolate->ThrowException(code(isolate, message));                          \
-  }                                                                           \
-  inline void THROW_ ## code(Environment* env, const char* message) {         \
-    THROW_ ## code(env->isolate(), message);                                  \
+#define V(code, type)                                                          \
+  template <typename... Args>                                                  \
+  inline v8::Local<v8::Value> code(                                            \
+      v8::Isolate* isolate, const char* format, Args&&... args) {              \
+    std::string message = SPrintF(format, std::forward<Args>(args)...);        \
+    v8::Local<v8::String> js_code = OneByteString(isolate, #code);             \
+    v8::Local<v8::String> js_msg =                                             \
+        OneByteString(isolate, message.c_str(), message.length());             \
+    v8::Local<v8::Object> e = v8::Exception::type(js_msg)                      \
+                                  ->ToObject(isolate->GetCurrentContext())     \
+                                  .ToLocalChecked();                           \
+    e->Set(isolate->GetCurrentContext(),                                       \
+           OneByteString(isolate, "code"),                                     \
+           js_code)                                                            \
+        .Check();                                                              \
+    return e;                                                                  \
+  }                                                                            \
+  template <typename... Args>                                                  \
+  inline void THROW_##code(                                                    \
+      v8::Isolate* isolate, const char* format, Args&&... args) {              \
+    isolate->ThrowException(                                                   \
+        code(isolate, format, std::forward<Args>(args)...));                   \
+  }                                                                            \
+  template <typename... Args>                                                  \
+  inline void THROW_##code(                                                    \
+      Environment* env, const char* format, Args&&... args) {                  \
+    THROW_##code(env->isolate(), format, std::forward<Args>(args)...);         \
   }
-  ERRORS_WITH_CODE(V)
+ERRORS_WITH_CODE(V)
 #undef V
 
 // Errors with predefined static messages