dawn.node: Fix rejecting promises in async pipeline compilation.
Previously the TypeError would be thrown immediately on
createPipelinAsync when it should reject the exception. Implement this
by allowing the converter to retain the exception instead of throwing
it, so that it can be acquired to reject the promise.
Also adds a bunch of [[nodiscard]].
Bug: None
Change-Id: Ib77480345e7862017fa03f6fab13b904a3c2e9a6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/159362
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/dawn/node/binding/Converter.cpp b/src/dawn/node/binding/Converter.cpp
index 2ba01a4..eb37ee7 100644
--- a/src/dawn/node/binding/Converter.cpp
+++ b/src/dawn/node/binding/Converter.cpp
@@ -44,6 +44,12 @@
for (auto& free : free_) {
free();
}
+ assert(exception == nullptr);
+}
+
+Napi::Error Converter::AcquireException() {
+ assert(exception != nullptr);
+ return std::move(exception);
}
bool Converter::HasFeature(wgpu::FeatureName feature) {
@@ -75,8 +81,7 @@
break;
}
}
- Napi::Error::New(env, "invalid value for GPUExtent3D").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUExtent3D");
}
bool Converter::Convert(wgpu::Origin3D& out, const interop::GPUOrigin3DDict& in) {
@@ -112,8 +117,7 @@
break;
}
}
- Napi::Error::New(env, "invalid value for GPUColor").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUColor");
}
bool Converter::Convert(wgpu::Origin3D& out, const std::vector<interop::GPUIntegerCoordinate>& in) {
@@ -145,8 +149,7 @@
out = wgpu::TextureAspect::DepthOnly;
return true;
}
- Napi::Error::New(env, "invalid value for GPUTextureAspect").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUTextureAspect");
}
bool Converter::Convert(wgpu::ImageCopyTexture& out, const interop::GPUImageCopyTexture& in) {
@@ -182,8 +185,7 @@
out.bytesPerElement = 1;
return true;
}
- Napi::Error::New(env, "invalid value for BufferSource").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for BufferSource");
}
bool Converter::Convert(wgpu::TextureDataLayout& out, const interop::GPUImageDataLayout& in) {
@@ -537,16 +539,12 @@
break;
default:
- Napi::Error::New(env, "invalid value for GPUTextureFormat")
- .ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUTextureFormat");
}
assert(requiredFeature != wgpu::FeatureName::Undefined);
if (!HasFeature(requiredFeature)) {
- Napi::TypeError::New(env, "invalid value for GPUTextureFormat")
- .ThrowAsJavaScriptException();
- return false;
+ return Throw(Napi::TypeError::New(env, "invalid value for GPUTextureFormat"));
}
return true;
@@ -720,8 +718,7 @@
out = wgpu::TextureDimension::e3D;
return true;
}
- Napi::Error::New(env, "invalid value for GPUTextureDimension").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUTextureDimension");
}
bool Converter::Convert(interop::GPUTextureDimension& out, wgpu::TextureDimension in) {
@@ -764,8 +761,7 @@
default:
break;
}
- Napi::Error::New(env, "invalid value for GPUTextureViewDimension").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUTextureViewDimension");
}
bool Converter::Convert(wgpu::ProgrammableStageDescriptor& out,
@@ -793,9 +789,7 @@
bool Converter::Convert(wgpu::ConstantEntry& out,
const std::string& in_name,
wgpu::interop::GPUPipelineConstantValue in_value) {
- out.key = in_name.c_str();
- out.value = in_value;
- return true;
+ return Convert(out.key, in_name) && Convert(out.value, in_value);
}
bool Converter::Convert(wgpu::BlendComponent& out, const interop::GPUBlendComponent& in) {
@@ -849,8 +843,7 @@
default:
break;
}
- Napi::Error::New(env, "invalid value for GPUBlendFactor").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUBlendFactor");
}
bool Converter::Convert(wgpu::BlendOperation& out, const interop::GPUBlendOperation& in) {
@@ -874,8 +867,7 @@
default:
break;
}
- Napi::Error::New(env, "invalid value for GPUBlendOperation").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUBlendOperation");
}
bool Converter::Convert(wgpu::BlendState& out, const interop::GPUBlendState& in) {
@@ -953,8 +945,7 @@
out = wgpu::PrimitiveTopology::TriangleStrip;
return true;
}
- Napi::Error::New(env, "invalid value for GPUPrimitiveTopology").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUPrimitiveTopology");
}
bool Converter::Convert(wgpu::FrontFace& out, const interop::GPUFrontFace& in) {
@@ -967,8 +958,7 @@
out = wgpu::FrontFace::CCW;
return true;
}
- Napi::Error::New(env, "invalid value for GPUFrontFace").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUFrontFace");
}
bool Converter::Convert(wgpu::CullMode& out, const interop::GPUCullMode& in) {
@@ -984,8 +974,7 @@
out = wgpu::CullMode::Back;
return true;
}
- Napi::Error::New(env, "invalid value for GPUCullMode").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUCullMode");
}
bool Converter::Convert(wgpu::CompareFunction& out, const interop::GPUCompareFunction& in) {
@@ -1016,8 +1005,7 @@
out = wgpu::CompareFunction::Always;
return true;
}
- Napi::Error::New(env, "invalid value for GPUCompareFunction").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUCompareFunction");
}
bool Converter::Convert(wgpu::IndexFormat& out, const interop::GPUIndexFormat& in) {
@@ -1030,8 +1018,7 @@
out = wgpu::IndexFormat::Uint32;
return true;
}
- Napi::Error::New(env, "invalid value for GPUIndexFormat").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUIndexFormat");
}
bool Converter::Convert(wgpu::StencilOperation& out, const interop::GPUStencilOperation& in) {
@@ -1062,8 +1049,7 @@
out = wgpu::StencilOperation::DecrementWrap;
return true;
}
- Napi::Error::New(env, "invalid value for GPUStencilOperation").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUStencilOperation");
}
bool Converter::Convert(wgpu::StencilFaceState& out, const interop::GPUStencilFaceState& in) {
@@ -1112,8 +1098,7 @@
default:
break;
}
- Napi::Error::New(env, "invalid value for GPUVertexStepMode").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUVertexStepMode");
}
bool Converter::Convert(wgpu::VertexAttribute& out, const interop::GPUVertexAttribute& in) {
@@ -1220,8 +1205,7 @@
default:
break;
}
- Napi::Error::New(env, "invalid value for GPUVertexFormat").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUVertexFormat");
}
bool Converter::Convert(wgpu::RenderPassColorAttachment& out,
@@ -1273,8 +1257,7 @@
out = wgpu::LoadOp::Clear;
return true;
}
- Napi::Error::New(env, "invalid value for GPULoadOp").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPULoadOp");
}
bool Converter::Convert(wgpu::StoreOp& out, const interop::GPUStoreOp& in) {
@@ -1287,8 +1270,7 @@
out = wgpu::StoreOp::Discard;
return true;
}
- Napi::Error::New(env, "invalid value for GPUStoreOp").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUStoreOp");
}
bool Converter::Convert(wgpu::BindGroupEntry& out, const interop::GPUBindGroupEntry& in) {
@@ -1316,9 +1298,7 @@
// TODO(crbug.com/dawn/1129): External textures
UNIMPLEMENTED(env, {});
}
- Napi::Error::New(env, "invalid value for GPUBindGroupEntry.resource")
- .ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUBindGroupEntry.resource");
}
bool Converter::Convert(wgpu::BindGroupLayoutEntry& out,
@@ -1364,8 +1344,7 @@
out = wgpu::BufferBindingType::ReadOnlyStorage;
return true;
}
- Napi::Error::New(env, "invalid value for GPUBufferBindingType").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUBufferBindingType");
}
bool Converter::Convert(wgpu::TextureSampleType& out, const interop::GPUTextureSampleType& in) {
@@ -1387,8 +1366,7 @@
out = wgpu::TextureSampleType::Uint;
return true;
}
- Napi::Error::New(env, "invalid value for GPUTextureSampleType").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUTextureSampleType");
}
bool Converter::Convert(wgpu::SamplerBindingType& out, const interop::GPUSamplerBindingType& in) {
@@ -1404,8 +1382,7 @@
out = wgpu::SamplerBindingType::Comparison;
return true;
}
- Napi::Error::New(env, "invalid value for GPUSamplerBindingType").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUSamplerBindingType");
}
bool Converter::Convert(wgpu::StorageTextureAccess& out,
@@ -1422,8 +1399,7 @@
out = wgpu::StorageTextureAccess::ReadWrite;
return true;
}
- Napi::Error::New(env, "invalid value for GPUStorageTextureAccess").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUStorageTextureAccess");
}
bool Converter::Convert(wgpu::QueryType& out, const interop::GPUQueryType& in) {
@@ -1437,13 +1413,10 @@
out = wgpu::QueryType::Timestamp;
return true;
} else {
- Napi::TypeError::New(env, "invalid value for GPUQueryType")
- .ThrowAsJavaScriptException();
- return false;
+ return Throw(Napi::TypeError::New(env, "invalid value for GPUQueryType"));
}
}
- Napi::Error::New(env, "invalid value for GPUQueryType").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUQueryType");
}
bool Converter::Convert(wgpu::FeatureName& out, interop::GPUFeatureName in) {
@@ -1595,8 +1568,7 @@
out = wgpu::AddressMode::MirrorRepeat;
return true;
}
- Napi::Error::New(env, "invalid value for GPUAddressMode").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUAddressMode");
}
bool Converter::Convert(wgpu::FilterMode& out, const interop::GPUFilterMode& in) {
@@ -1609,8 +1581,7 @@
out = wgpu::FilterMode::Linear;
return true;
}
- Napi::Error::New(env, "invalid value for GPUFilterMode").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUFilterMode");
}
bool Converter::Convert(wgpu::MipmapFilterMode& out, const interop::GPUMipmapFilterMode& in) {
@@ -1623,8 +1594,7 @@
out = wgpu::MipmapFilterMode::Linear;
return true;
}
- Napi::Error::New(env, "invalid value for GPUMipmapFilterMode").ThrowAsJavaScriptException();
- return false;
+ return Throw("invalid value for GPUMipmapFilterMode");
}
bool Converter::Convert(wgpu::ComputePipelineDescriptor& out,
@@ -1655,4 +1625,19 @@
return true;
}
+bool Converter::Throw(std::string&& message) {
+ return Throw(Napi::Error::New(env, message));
+}
+
+bool Converter::Throw(Napi::Error&& error) {
+ if (retainException) {
+ assert(exception == nullptr);
+ exception = std::move(error);
+ } else {
+ error.ThrowAsJavaScriptException();
+ }
+
+ return false;
+}
+
} // namespace wgpu::binding
diff --git a/src/dawn/node/binding/Converter.h b/src/dawn/node/binding/Converter.h
index 0e63431..657e322 100644
--- a/src/dawn/node/binding/Converter.h
+++ b/src/dawn/node/binding/Converter.h
@@ -82,8 +82,8 @@
class Converter {
public:
explicit Converter(Napi::Env e) : env(e) {}
- Converter(Napi::Env e, wgpu::Device extensionDevice)
- : env(e), device(std::move(extensionDevice)) {}
+ Converter(Napi::Env e, wgpu::Device extensionDevice, bool retainExceptionIn = false)
+ : env(e), device(std::move(extensionDevice)), retainException(retainExceptionIn) {}
~Converter();
// Conversion function. Converts the interop type IN to the Dawn type OUT.
@@ -108,6 +108,10 @@
// Returns the Env that this Converter was constructed with.
inline Napi::Env Env() const { return env; }
+ // Returns the Napi::Error stored from previous conversion and clears it. (Converter checks any
+ // retained exception is acquired)
+ Napi::Error AcquireException();
+
// BufferSource is the converted type of interop::BufferSource.
struct BufferSource {
void* data;
@@ -288,7 +292,7 @@
[[nodiscard]] bool Convert(interop::GPUFeatureName& out, wgpu::FeatureName in);
// std::string to C string
- inline bool Convert(const char*& out, const std::string& in) {
+ [[nodiscard]] inline bool Convert(const char*& out, const std::string& in) {
out = in.c_str();
return true;
}
@@ -307,38 +311,36 @@
inline bool Convert(OUT& out, const IN& in) {
out = static_cast<OUT>(in);
if (static_cast<IN>(out) != in) {
- Napi::Error::New(env, "Integer value (" + std::to_string(in) +
- ") cannot be converted to the Dawn data type without "
- "truncation of the value")
- .ThrowAsJavaScriptException();
- return false;
+ return Throw("Integer value (" + std::to_string(in) +
+ ") cannot be converted to the Dawn data type without "
+ "truncation of the value");
}
return true;
}
// ClampedInteger<T>
template <typename T>
- inline bool Convert(T& out, const interop::ClampedInteger<T>& in) {
+ [[nodiscard]] inline bool Convert(T& out, const interop::ClampedInteger<T>& in) {
out = in;
return true;
}
// EnforceRangeInteger<T>
template <typename T>
- inline bool Convert(T& out, const interop::EnforceRangeInteger<T>& in) {
+ [[nodiscard]] inline bool Convert(T& out, const interop::EnforceRangeInteger<T>& in) {
out = in;
return true;
}
template <typename OUT, typename... IN_TYPES>
- inline bool Convert(OUT& out, const std::variant<IN_TYPES...>& in) {
+ [[nodiscard]] inline bool Convert(OUT& out, const std::variant<IN_TYPES...>& in) {
return std::visit([&](auto&& i) { return Convert(out, i); }, in);
}
// If the std::optional does not have a value, then Convert() simply returns true and 'out'
// is not assigned a new value.
template <typename OUT, typename IN>
- inline bool Convert(OUT& out, const std::optional<IN>& in) {
+ [[nodiscard]] inline bool Convert(OUT& out, const std::optional<IN>& in) {
if (in.has_value()) {
return Convert(out, in.value());
}
@@ -351,7 +353,7 @@
template <typename OUT,
typename IN,
typename _ = std::enable_if_t<!std::is_same_v<IN, std::string>>>
- inline bool Convert(OUT*& out, const std::optional<IN>& in) {
+ [[nodiscard]] inline bool Convert(OUT*& out, const std::optional<IN>& in) {
if (in.has_value()) {
auto* el = Allocate<std::remove_const_t<OUT>>();
if (!Convert(*el, in.value())) {
@@ -366,7 +368,7 @@
// interop::Interface -> Dawn object
template <typename OUT, typename IN>
- inline bool Convert(OUT& out, const interop::Interface<IN>& in) {
+ [[nodiscard]] inline bool Convert(OUT& out, const interop::Interface<IN>& in) {
using Impl = ImplOf<IN>;
out = *in.template As<Impl>();
if (!out) {
@@ -378,7 +380,7 @@
// vector -> raw pointer + count
template <typename OUT, typename IN>
- inline bool Convert(OUT*& out_els, size_t& out_count, const std::vector<IN>& in) {
+ [[nodiscard]] inline bool Convert(OUT*& out_els, size_t& out_count, const std::vector<IN>& in) {
if (in.size() == 0) {
out_els = nullptr;
out_count = 0;
@@ -396,9 +398,9 @@
// unordered_map -> raw pointer + count
template <typename OUT, typename IN_KEY, typename IN_VALUE>
- inline bool Convert(OUT*& out_els,
- size_t& out_count,
- const std::unordered_map<IN_KEY, IN_VALUE>& in) {
+ [[nodiscard]] inline bool Convert(OUT*& out_els,
+ size_t& out_count,
+ const std::unordered_map<IN_KEY, IN_VALUE>& in) {
if (in.size() == 0) {
out_els = nullptr;
out_count = 0;
@@ -417,7 +419,9 @@
// std::optional<T> -> raw pointer + count
template <typename OUT, typename IN>
- inline bool Convert(OUT*& out_els, size_t& out_count, const std::optional<IN>& in) {
+ [[nodiscard]] inline bool Convert(OUT*& out_els,
+ size_t& out_count,
+ const std::optional<IN>& in) {
if (!in.has_value()) {
out_els = nullptr;
out_count = 0;
@@ -429,8 +433,24 @@
Napi::Env env;
wgpu::Device device = nullptr;
+ bool retainException = false;
+ Napi::Error exception;
+
bool HasFeature(wgpu::FeatureName feature);
+ // Function to be used when throwing a JavaScript exception because of issues during the
+ // conversion. Because the conversion should stop immediately, this method returns false,
+ // so the pattern is:
+ //
+ // if (some error case) {
+ // return Throw("Some error case description");
+ // }
+ //
+ // A variant also exists that doesn't take a string_view, so that more specific error types
+ // can be thrown.
+ [[nodiscard]] bool Throw(std::string&& message);
+ [[nodiscard]] bool Throw(Napi::Error&& error);
+
// Allocate() allocates and constructs an array of 'n' elements, and returns a pointer to
// the first element. The array is freed when the Converter is destructed.
template <typename T>
diff --git a/src/dawn/node/binding/GPUDevice.cpp b/src/dawn/node/binding/GPUDevice.cpp
index 7a173d4..008f2b7 100644
--- a/src/dawn/node/binding/GPUDevice.cpp
+++ b/src/dawn/node/binding/GPUDevice.cpp
@@ -377,12 +377,12 @@
interop::GPUComputePipelineDescriptor descriptor) {
using Promise = interop::Promise<interop::Interface<interop::GPUComputePipeline>>;
- Converter conv(env);
+ Converter conv(env, device_, true);
wgpu::ComputePipelineDescriptor desc{};
if (!conv(desc, descriptor)) {
Promise promise(env, PROMISE_INFO);
- promise.Reject(Errors::OperationError(env));
+ promise.Reject(conv.AcquireException());
return promise;
}
@@ -422,12 +422,12 @@
interop::GPURenderPipelineDescriptor descriptor) {
using Promise = interop::Promise<interop::Interface<interop::GPURenderPipeline>>;
- Converter conv(env, device_);
+ Converter conv(env, device_, true);
wgpu::RenderPipelineDescriptor desc{};
if (!conv(desc, descriptor)) {
Promise promise(env, PROMISE_INFO);
- promise.Reject(Errors::OperationError(env));
+ promise.Reject(conv.AcquireException());
return promise;
}
