From d59c6de7e801639573f6c210ba715a9298a2b932 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Wed, 3 Mar 2021 22:17:11 -0600 Subject: [PATCH] src: add error formatting support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/37598 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Darshan Sen Reviewed-By: Michaël Zasso --- doc/api/errors.md | 5 ++++ lib/internal/vm/module.js | 2 ++ src/module_wrap.cc | 24 +++++++++++-------- src/node_errors.h | 50 ++++++++++++++++++++++++--------------- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 96e810d878653a..453c0616935947 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2313,6 +2313,11 @@ than the parent module. Linked modules must share the same context. The linker function returned a module for which linking has failed. + +### `ERR_VM_MODULE_LINK_FAILURE` + +The module was unable to be linked due to a failure. + ### `ERR_VM_MODULE_NOT_MODULE` diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index 56164b0d8b98d4..3052d21e1a42fe 100644 --- 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 index 4885e65f9c5d47..ef20bbe19dd6af 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -291,7 +291,9 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { Local 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 resolve_promise = resolve_return_value.As(); obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise); @@ -485,17 +487,19 @@ MaybeLocal ModuleWrap::ResolveCallback(Local context, 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(); } - 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(); } @@ -503,15 +507,15 @@ MaybeLocal ModuleWrap::ResolveCallback(Local context, 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(); } Local module_object = resolve_promise->Result().As(); 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(); } diff --git a/src/node_errors.h b/src/node_errors.h index 984603c42e2618..1ae461f23d336f 100644 --- 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 code(v8::Isolate* isolate, \ - const char* message) { \ - v8::Local js_code = OneByteString(isolate, #code); \ - v8::Local js_msg = OneByteString(isolate, message); \ - v8::Local 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 \ + inline v8::Local code( \ + v8::Isolate* isolate, const char* format, Args&&... args) { \ + std::string message = SPrintF(format, std::forward(args)...); \ + v8::Local js_code = OneByteString(isolate, #code); \ + v8::Local js_msg = \ + OneByteString(isolate, message.c_str(), message.length()); \ + v8::Local e = v8::Exception::type(js_msg) \ + ->ToObject(isolate->GetCurrentContext()) \ + .ToLocalChecked(); \ + e->Set(isolate->GetCurrentContext(), \ + OneByteString(isolate, "code"), \ + js_code) \ + .Check(); \ + return e; \ + } \ + template \ + inline void THROW_##code( \ + v8::Isolate* isolate, const char* format, Args&&... args) { \ + isolate->ThrowException( \ + code(isolate, format, std::forward(args)...)); \ + } \ + template \ + inline void THROW_##code( \ + Environment* env, const char* format, Args&&... args) { \ + THROW_##code(env->isolate(), format, std::forward(args)...); \ } - ERRORS_WITH_CODE(V) +ERRORS_WITH_CODE(V) #undef V // Errors with predefined static messages