Skip to content

Commit

Permalink
fix: Fix Promise rejections not having a message by using `std::excep…
Browse files Browse the repository at this point in the history 
…tion_ptr` to avoid C++ object slicing (#369)

* fix: Use `std::exception_ptr` to preserve message

* fix: Catch exception

* fix: Fix `JSIConverter`

* Update AssertPromiseState.hpp

* Ok kinda fix?

* ugh

* fix again

* Update JSIConverter+Promise.hpp

* chore: Rename `_result` to `_state`

* feat: Allow comparing message

* fix: Fix `RuntimeError.description`

* feat: Test all of those error messages as well
  • Loading branch information
@mrousavy
mrousavy authored Nov 22, 2024
1 parent 097facd commit b069fb6
Show file tree
Hide file tree
Showing 19 changed files with 202 additions and 155 deletions.
10 changes: 8 additions & 2 deletions example/src/Testers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@ export class State<T> {
throw new Error(reason)
}

didThrow(): State<T> {
didThrow(message?: string): State<T> {
if (this.errorThrown == null) {
this.onFailed(
`Expected test to throw an error, but no error was thrown! Instead it returned a result: ${stringify(this.result)}`
)
} else {
this.onPassed()
if (message == null || stringify(this.errorThrown) === message) {
this.onPassed()
} else {
this.onFailed(
`Expected test to throw "${message}", but it threw a different error: "${stringify(this.errorThrown)}"`
)
}
}
return this
}
Expand Down
45 changes: 34 additions & 11 deletions example/src/getTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,9 @@ export function getTests(

// Test errors
createTest('funcThatThrows()', () =>
it(() => testObject.funcThatThrows()).didThrow()
it(() => testObject.funcThatThrows()).didThrow(
`Error: ${testObject.name}.funcThatThrows(...): This function will only work after sacrificing seven lambs!`
)
),

// Optional parameters
Expand All @@ -449,13 +451,17 @@ export function getTests(
// @ts-expect-error
'too many args!'
)
).didThrow()
).didThrow(
`Error: \`${testObject.name}.tryOptionalParams(...)\` expected between 2 and 3 arguments, but received 4!`
)
),
createTest('tryOptionalParams(...) one-too-few', () =>
it(() =>
// @ts-expect-error
testObject.tryOptionalParams(13)
).didThrow()
).didThrow(
`Error: \`${testObject.name}.tryOptionalParams(...)\` expected between 2 and 3 arguments, but received 1!`
)
),
createTest('tryMiddleParam(...)', () =>
it(() => testObject.tryMiddleParam(13, undefined, 'hello!'))
Expand Down Expand Up @@ -502,7 +508,9 @@ export function getTests(
() =>
// @ts-expect-error
(testObject.someVariant = false)
).didThrow()
).didThrow(
`Error: ${testObject.name}.someVariant: Cannot convert "false" to any type in variant<std::string, double>!`
)
),

// More complex variants...
Expand Down Expand Up @@ -544,7 +552,9 @@ export function getTests(
),
createTest('getVariantEnum(...) throws at wrong type (string)', () =>
// @ts-expect-error
it(() => testObject.getVariantEnum('string')).didThrow()
it(() => testObject.getVariantEnum('string')).didThrow(
`Error: ${testObject.name}.getVariantEnum(...): Cannot convert "string" to any type in variant<bool, margelo::nitro::image::OldEnum>!`
)
),
createTest('getVariantObjects(...) converts Person', () =>
it(() => testObject.getVariantObjects(TEST_PERSON))
Expand All @@ -567,15 +577,19 @@ export function getTests(
'getVariantObjects(...) throws at wrong type (string)',
() =>
// @ts-expect-error
it(() => testObject.getVariantObjects('some-string')).didThrow()
it(() => testObject.getVariantObjects('some-string')).didThrow(
`Error: ${testObject.name}.getVariantObjects(...): Cannot convert "some-string" to any type in variant<margelo::nitro::image::Car, margelo::nitro::image::Person>!`
)
),
createTest(
'getVariantObjects(...) throws at wrong type (wrong object)',
() =>
it(() =>
// @ts-expect-error
testObject.getVariantObjects({ someValue: 55 })
).didThrow()
).didThrow(
`Error: ${testObject.name}.getVariantObjects(...): Cannot convert "[object Object]" to any type in variant<margelo::nitro::image::Car, margelo::nitro::image::Person>!`
)
),
createTest('getVariantHybrid(...) converts Hybrid', () =>
it(() => testObject.getVariantHybrid(testObject))
Expand All @@ -592,15 +606,19 @@ export function getTests(
'getVariantHybrid(...) throws at wrong type (string)',
() =>
// @ts-expect-error
it(() => testObject.getVariantHybrid('some-string')).didThrow()
it(() => testObject.getVariantHybrid('some-string')).didThrow(
`Error: ${testObject.name}.getVariantHybrid(...): Cannot convert "some-string" to any type in variant<std::shared_ptr<margelo::nitro::image::HybridTestObjectCppSpec>, margelo::nitro::image::Person>!`
)
),
createTest(
'getVariantHybrid(...) throws at wrong type (wrong object)',
() =>
it(() =>
// @ts-expect-error
testObject.getVariantHybrid({ someValue: 55 })
).didThrow()
).didThrow(
`Error: ${testObject.name}.getVariantHybrid(...): Cannot convert "[object Object]" to any type in variant<std::shared_ptr<margelo::nitro::image::HybridTestObjectCppSpec>, margelo::nitro::image::Person>!`
)
),
createTest('getVariantTuple(...) converts Float2', () =>
it(() => testObject.getVariantTuple([10, 20]))
Expand Down Expand Up @@ -660,7 +678,9 @@ export function getTests(
// @ts-expect-error
[10, 20]
)
).didThrow()
).didThrow(
`Error: ${testObject.name}.flip(...): The given JS Array has 2 items, but std::tuple<double, double, double> expects 3 items.`
)
),
createTest('passTuple(...)', () =>
it(() => testObject.passTuple([13, 'hello', true]))
Expand All @@ -687,7 +707,9 @@ export function getTests(
.equals(55n)
),
createTest('promiseThrows() throws', async () =>
(await it(() => testObject.promiseThrows())).didThrow()
(await it(() => testObject.promiseThrows())).didThrow(
'Error: Promise throws :)'
)
),
createTest('twoPromises can run in parallel', async () =>
(
Expand Down Expand Up @@ -1073,6 +1095,7 @@ export function getTests(
// @ts-expect-error
testObject.bounceChild(child)
} else {
// This only throws in __DEV__ - in release it is optimized away and would crash. :)
throw new Error(
`This only throws in __DEV__ - in release it is optimized away and would crash. :)`
)
Expand Down
8 changes: 4 additions & 4 deletions packages/nitrogen/src/syntax/kotlin/KotlinCxxBridgedType.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,8 @@ export class KotlinCxxBridgedType implements BridgedType<'kotlin', 'c++'> {
${parameterName}->addOnResolvedListener([=]() {
__promise->cthis()->resolve(JUnit::instance());
});
${parameterName}->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
${parameterName}->addOnRejectedListener([=](const std::exception_ptr& __error) {
auto __jniError = jni::getJavaExceptionForCppException(__error);
__promise->cthis()->reject(__jniError);
});
return __promise;
Expand All @@ -550,8 +550,8 @@ export class KotlinCxxBridgedType implements BridgedType<'kotlin', 'c++'> {
${parameterName}->addOnResolvedListener([=](const ${resolvingType}& __result) {
__promise->cthis()->resolve(${indent(bridge.parseFromCppToKotlin('__result', 'c++', true), ' ')});
});
${parameterName}->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
${parameterName}->addOnRejectedListener([=](const std::exception_ptr& __error) {
auto __jniError = jni::getJavaExceptionForCppException(__error);
__promise->cthis()->reject(__jniError);
});
return __promise;
Expand Down
4 changes: 2 additions & 2 deletions packages/nitrogen/src/syntax/swift/SwiftCxxBridgedType.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
{ () -> ${promise.getCode('swift')} in
let __promise = ${promise.getCode('swift')}()
let __resolver = SwiftClosure { __promise.resolve(withResult: ()) }
let __rejecter = { (__error: std.exception) in
let __rejecter = { (__error: std.exception_ptr) in
__promise.reject(withError: RuntimeError.from(cppError: __error))
}
let __resolverCpp = __resolver.getFunctionCopy()
Expand Down Expand Up @@ -371,7 +371,7 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
let __resolver = { (__result: ${resolvingTypeBridge.getTypeCode('swift')}) in
__promise.resolve(withResult: ${indent(resolvingTypeBridge.parseFromCppToSwift('__result', 'swift'), ' ')})
}
let __rejecter = { (__error: std.exception) in
let __rejecter = { (__error: std.exception_ptr) in
__promise.reject(withError: RuntimeError.from(cppError: __error))
}
let __resolverCpp = ${indent(resolverFuncBridge.parseFromSwiftToCpp('__resolver', 'swift'), ' ')}
Expand Down
4 changes: 2 additions & 2 deletions packages/nitrogen/src/syntax/types/ErrorType.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ export class ErrorType implements Type {
getCode(language: Language): string {
switch (language) {
case 'c++':
return `std::exception`
return `std::exception_ptr`
case 'swift':
return `std.exception`
return `std.exception_ptr`
case 'kotlin':
return `Throwable`
default:
Expand Down
6 changes: 3 additions & 3 deletions packages/react-native-nitro-image/cpp/HybridTestObjectCpp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,21 +279,21 @@ std::shared_ptr<Promise<double>> HybridTestObjectCpp::getValueFromJSCallbackAndW
std::shared_ptr<Promise<double>> HybridTestObjectCpp::awaitAndGetPromise(const std::shared_ptr<Promise<double>>& promise) {
auto newPromise = Promise<double>::create();
promise->addOnResolvedListener([=](double result) { newPromise->resolve(result); });
promise->addOnRejectedListener([=](const std::exception& error) { newPromise->reject(error); });
promise->addOnRejectedListener([=](const std::exception_ptr& error) { newPromise->reject(error); });
return newPromise;
}

std::shared_ptr<Promise<Car>> HybridTestObjectCpp::awaitAndGetComplexPromise(const std::shared_ptr<Promise<Car>>& promise) {
auto newPromise = Promise<Car>::create();
promise->addOnResolvedListener([=](const Car& result) { newPromise->resolve(result); });
promise->addOnRejectedListener([=](const std::exception& error) { newPromise->reject(error); });
promise->addOnRejectedListener([=](const std::exception_ptr& error) { newPromise->reject(error); });
return newPromise;
}

std::shared_ptr<Promise<void>> HybridTestObjectCpp::awaitPromise(const std::shared_ptr<Promise<void>>& promise) {
auto newPromise = Promise<void>::create();
promise->addOnResolvedListener([=]() { newPromise->resolve(); });
promise->addOnRejectedListener([=](const std::exception& error) { newPromise->reject(error); });
promise->addOnRejectedListener([=](const std::exception_ptr& error) { newPromise->reject(error); });
return newPromise;
}

Expand Down
12 changes: 6 additions & 6 deletions ...ct-native-nitro-image/nitrogen/generated/android/c++/JHybridTestObjectSwiftKotlinSpec.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,8 @@ namespace margelo::nitro::image {
promise->addOnResolvedListener([=](const double& __result) {
__promise->cthis()->resolve(jni::JDouble::valueOf(__result));
});
promise->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
promise->addOnRejectedListener([=](const std::exception_ptr& __error) {
auto __jniError = jni::getJavaExceptionForCppException(__error);
__promise->cthis()->reject(__jniError);
});
return __promise;
Expand All @@ -439,8 +439,8 @@ namespace margelo::nitro::image {
promise->addOnResolvedListener([=](const Car& __result) {
__promise->cthis()->resolve(JCar::fromCpp(__result));
});
promise->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
promise->addOnRejectedListener([=](const std::exception_ptr& __error) {
auto __jniError = jni::getJavaExceptionForCppException(__error);
__promise->cthis()->reject(__jniError);
});
return __promise;
Expand All @@ -465,8 +465,8 @@ namespace margelo::nitro::image {
promise->addOnResolvedListener([=]() {
__promise->cthis()->resolve(JUnit::instance());
});
promise->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
promise->addOnRejectedListener([=](const std::exception_ptr& __error) {
auto __jniError = jni::getJavaExceptionForCppException(__error);
__promise->cthis()->reject(__jniError);
});
return __promise;
Expand Down
26 changes: 13 additions & 13 deletions packages/react-native-nitro-image/nitrogen/generated/ios/NitroImage-Swift-Cxx-Bridge.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,32 +314,32 @@ namespace margelo::nitro::image::bridge::swift {
return std::make_shared<Func_void_int64_t_Wrapper>(value);
}

// pragma MARK: std::function<void(const std::exception& /* error */)>
// pragma MARK: std::function<void(const std::exception_ptr& /* error */)>
/**
* Specialized version of `std::function<void(const std::exception&)>`.
* Specialized version of `std::function<void(const std::exception_ptr&)>`.
*/
using Func_void_std__exception = std::function<void(const std::exception& /* error */)>;
using Func_void_std__exception_ptr = std::function<void(const std::exception_ptr& /* error */)>;
/**
* Wrapper class for a `std::function<void(const std::exception& / * error * /)>`, this can be used from Swift.
* Wrapper class for a `std::function<void(const std::exception_ptr& / * error * /)>`, this can be used from Swift.
*/
class Func_void_std__exception_Wrapper final {
class Func_void_std__exception_ptr_Wrapper final {
public:
explicit Func_void_std__exception_Wrapper(const std::function<void(const std::exception& /* error */)>& func): _function(func) {}
explicit Func_void_std__exception_Wrapper(std::function<void(const std::exception& /* error */)>&& func): _function(std::move(func)) {}
inline void call(std::exception error) const {
explicit Func_void_std__exception_ptr_Wrapper(const std::function<void(const std::exception_ptr& /* error */)>& func): _function(func) {}
explicit Func_void_std__exception_ptr_Wrapper(std::function<void(const std::exception_ptr& /* error */)>&& func): _function(std::move(func)) {}
inline void call(std::exception_ptr error) const {
_function(error);
}
private:
std::function<void(const std::exception& /* error */)> _function;
std::function<void(const std::exception_ptr& /* error */)> _function;
};
inline Func_void_std__exception create_Func_void_std__exception(void* _Nonnull closureHolder, void(* _Nonnull call)(void* _Nonnull /* closureHolder */, std::exception), void(* _Nonnull destroy)(void* _Nonnull)) {
inline Func_void_std__exception_ptr create_Func_void_std__exception_ptr(void* _Nonnull closureHolder, void(* _Nonnull call)(void* _Nonnull /* closureHolder */, std::exception_ptr), void(* _Nonnull destroy)(void* _Nonnull)) {
std::shared_ptr<void> sharedClosureHolder(closureHolder, destroy);
return Func_void_std__exception([sharedClosureHolder, call](const std::exception& error) -> void {
return Func_void_std__exception_ptr([sharedClosureHolder, call](const std::exception_ptr& error) -> void {
call(sharedClosureHolder.get(), error);
});
}
inline std::shared_ptr<Func_void_std__exception_Wrapper> share_Func_void_std__exception(const Func_void_std__exception& value) {
return std::make_shared<Func_void_std__exception_Wrapper>(value);
inline std::shared_ptr<Func_void_std__exception_ptr_Wrapper> share_Func_void_std__exception_ptr(const Func_void_std__exception_ptr& value) {
return std::make_shared<Func_void_std__exception_ptr_Wrapper>(value);
}

// pragma MARK: std::shared_ptr<Promise<void>>
Expand Down
Loading

0 comments on commit b069fb6

Please sign in to comment.