Skip to content

Commit

Permalink
feat: Implement Swift errors 🥳 (#430)
Browse files Browse the repository at this point in the history
* chore: Use C++ 23

* feat: Add `ResultWrappingType`

* feat: Add C++ `ResultWrappingType` bridge

* feat: Use C++ result wrapper

* feat: Use `std::expected` for Swift return types

* Bump to 23

* Downgrade to C++ 20 again

* fix: Use `Result<T>` instead of expected

* fix: Return even if void

* fix: Explicitly say type

* extra line for debugging

* fix: Fix double args

* fix: Bridge directly to C++ target for return values

* feat: THROW SWIFT ERROR WOOHOOO

* feat: Make it work

* feat: Add test that uses `Result<complex type>` and throws before

* Update getTests.ts

* Update HybridTestObjectKotlin.kt

* extra line

* perf: Use `assert` for `value()`

* docs: Remove error swift limitation as it now works
  • Loading branch information
mrousavy authored Dec 18, 2024
1 parent 5b510b8 commit 5b3fa6c
Show file tree
Hide file tree
Showing 32 changed files with 1,237 additions and 389 deletions.
5 changes: 0 additions & 5 deletions docs/docs/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,3 @@ try {
console.log(error)
}
```

## Swift Errors

Due to a Swift compiler bug, Swift Hybrid Objects can currently not throw errors. Instead, they will raise a `fatalError(..)` which can only be seen if the app is running in Xcode (debugger).
This should be fixed in the next Xcode version.
2 changes: 1 addition & 1 deletion example/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1895,7 +1895,7 @@ SPEC CHECKSUMS:
glog: 08b301085f15bcbb6ff8632a8ebaf239aae04e6a
hermes-engine: 06a9c6900587420b90accc394199527c64259db4
NitroImage: df6e7bc6d5cc2be5d3c47aec9e88d2af8498da88
NitroModules: d5e3886c34fc5c5ebab18327e52f35460c039e2d
NitroModules: bc157840c2fbc711c282025d3964b16d857cb9eb
RCT-Folly: bf5c0376ffe4dd2cf438dcf86db385df9fdce648
RCTDeprecation: fb7d408617e25d7f537940000d766d60149c5fea
RCTRequired: 9aaf0ffcc1f41f0c671af863970ef25c422a9920
Expand Down
7 changes: 7 additions & 0 deletions example/src/getTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,13 @@ export function getTests(
`Error: ${testObject.name}.funcThatThrows(...): This function will only work after sacrificing seven lambs!`
)
),
createTest('funcThatThrowsBeforePromise() throws', async () =>
(
await it(async () => await testObject.funcThatThrowsBeforePromise())
).didThrow(
`Error: ${testObject.name}.funcThatThrowsBeforePromise(...): This function will only work after sacrificing eight lambs!`
)
),
createTest('throwError(error) throws same message from JS', () =>
it(() => {
const error = new Error('rethrowing a JS error from native!')
Expand Down
12 changes: 12 additions & 0 deletions packages/nitrogen/src/syntax/createType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,18 @@ function getTypeId(type: TSMorphType, isOptional: boolean): string {
return key
}

export function addKnownType(
key: string,
type: Type,
language: Language
): void {
if (knownTypes[language].has(key)) {
// type is already known
return
}
knownTypes[language].set(key, type)
}

/**
* Create a new type (or return it from cache if it is already known)
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/nitrogen/src/syntax/swift/SwiftCxxBridgedType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
case 'map':
// AnyMapHolder <> AnyMap
return true
case 'result-wrapper':
// Result<T> <> T
return true
default:
return false
}
Expand Down Expand Up @@ -223,6 +226,7 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
case 'variant':
case 'tuple':
case 'record':
case 'result-wrapper':
case 'promise': {
const bridge = this.getBridgeOrThrow()
switch (language) {
Expand Down
56 changes: 56 additions & 0 deletions packages/nitrogen/src/syntax/swift/SwiftCxxTypeHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { getUmbrellaHeaderName } from '../../autolinking/ios/createSwiftUmbrella
import { VoidType } from '../types/VoidType.js'
import { NamedWrappingType } from '../types/NamedWrappingType.js'
import { ErrorType } from '../types/ErrorType.js'
import { ResultWrappingType } from '../types/ResultWrappingType.js'

export interface SwiftCxxHelper {
cxxHeader: {
Expand Down Expand Up @@ -52,6 +53,10 @@ export function createSwiftCxxHelpers(type: Type): SwiftCxxHelper | undefined {
return createCxxTupleSwiftHelper(getTypeAs(type, TupleType))
case 'promise':
return createCxxPromiseSwiftHelper(getTypeAs(type, PromiseType))
case 'result-wrapper':
return createCxxResultWrapperSwiftHelper(
getTypeAs(type, ResultWrappingType)
)
default:
return undefined
}
Expand Down Expand Up @@ -504,6 +509,57 @@ inline ${actualType} create_${name}(${typesSignature}) {
}
}

/**
* Create a C++ `create_result` function that can be called from Swift to create a `Result<T>`.
*/
function createCxxResultWrapperSwiftHelper(
type: ResultWrappingType
): SwiftCxxHelper {
const actualType = type.getCode('c++')
const name = escapeCppName(type.getCode('c++'))
const funcName = `create_${name}`

const functions: string[] = []
if (type.result.kind === 'void') {
functions.push(
`
inline ${name} ${funcName}() {
return ${actualType}::withValue();
}`.trim()
)
} else {
const typeParam = type.result.canBePassedByReference
? `const ${type.result.getCode('c++')}&`
: type.result.getCode('c++')
functions.push(
`
inline ${name} ${funcName}(${typeParam} value) {
return ${actualType}::withValue(${type.result.canBePassedByReference ? 'value' : 'std::move(value)'});
}`.trim()
)
}
functions.push(
`
inline ${name} ${funcName}(const ${type.error.getCode('c++')}& error) {
return ${actualType}::withError(error);
}`.trim()
)

return {
cxxType: actualType,
specializationName: name,
funcName: funcName,
cxxHeader: {
code: `
using ${name} = ${actualType};
${functions.join('\n')}
`.trim(),
requiredIncludes: type.getRequiredImports(),
},
dependencies: [],
}
}

/**
* Creates a C++ `create_promise_T()` function that can be called from Swift to create a `std::shared_ptr<Promise<T>>`.
*/
Expand Down
57 changes: 43 additions & 14 deletions packages/nitrogen/src/syntax/swift/SwiftHybridObjectBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { NitroConfig } from '../../config/NitroConfig.js'
import { includeHeader } from '../c++/includeNitroHeader.js'
import { getUmbrellaHeaderName } from '../../autolinking/ios/createSwiftUmbrellaHeader.js'
import { HybridObjectType } from '../types/HybridObjectType.js'
import { addKnownType } from '../createType.js'
import { ResultWrappingType } from '../types/ResultWrappingType.js'

export function getBridgeNamespace() {
return NitroConfig.getCxxNamespace('swift', 'bridge', 'swift')
Expand Down Expand Up @@ -231,19 +233,26 @@ return ${bridged.parseFromSwiftToCpp('__result', 'c++')};
}
})
.join(', ')
const bridgedReturnType = new SwiftCxxBridgedType(m.returnType)
const bridgedReturnType = new SwiftCxxBridgedType(m.returnType, true)
const hasResult = m.returnType.kind !== 'void'
let body: string
if (hasResult) {
// func returns something
body = `
auto __result = _swiftPart.${m.name}(${params});
return ${bridgedReturnType.parseFromSwiftToCpp('__result', 'c++')};
if (__result.hasError()) [[unlikely]] {
std::rethrow_exception(__result.error());
}
auto __value = std::move(__result.value());
return ${bridgedReturnType.parseFromSwiftToCpp('__value', 'c++')};
`.trim()
} else {
// void func
body = `
_swiftPart.${m.name}(${params});
auto __result = _swiftPart.${m.name}(${params});
if (__result.hasError()) [[unlikely]] {
std::rethrow_exception(__result.error());
}
`.trim()
}

Expand Down Expand Up @@ -419,7 +428,18 @@ public var ${property.name}: ${bridgedType.getTypeCode('swift')} {
}

function getMethodForwardImplementation(method: Method): string {
const returnType = new SwiftCxxBridgedType(method.returnType)
// wrapped return in a std::expected
const resultType = new ResultWrappingType(method.returnType)
addKnownType(`expected_${resultType.getCode('c++')}`, resultType, 'swift')
const bridgedResultType = new SwiftCxxBridgedType(resultType, true)
const resultBridge = bridgedResultType.getRequiredBridge()
if (resultBridge == null)
throw new Error(
`Result type (${bridgedResultType.getTypeCode('c++')}) does not have a bridge!`
)
const bridgedErrorType = new SwiftCxxBridgedType(resultType.error, true)

const returnType = new SwiftCxxBridgedType(method.returnType, true)
const params = method.parameters.map((p) => {
const bridgedType = new SwiftCxxBridgedType(p.type)
return `${p.name}: ${bridgedType.getTypeCode('swift')}`
Expand All @@ -428,19 +448,28 @@ function getMethodForwardImplementation(method: Method): string {
const bridgedType = new SwiftCxxBridgedType(p.type)
return `${p.name}: ${bridgedType.parseFromCppToSwift(p.name, 'swift')}`
})
const resultValue = returnType.hasType ? `let __result = ` : ''
const returnValue = returnType.hasType
? `${returnType.parseFromSwiftToCpp('__result', 'swift')}`
: ''
let body: string
if (returnType.hasType) {
body = `
let __result = try self.__implementation.${method.name}(${passParams.join(', ')})
let __resultCpp = ${returnType.parseFromSwiftToCpp('__result', 'swift')}
return bridge.${resultBridge.funcName}(__resultCpp)
`.trim()
} else {
body = `
try self.__implementation.${method.name}(${passParams.join(', ')})
return bridge.${resultBridge.funcName}()
`.trim()
}

return `
@inline(__always)
public func ${method.name}(${params.join(', ')}) -> ${returnType.getTypeCode('swift')} {
public func ${method.name}(${params.join(', ')}) -> ${bridgedResultType.getTypeCode('swift')} {
do {
${resultValue}try self.__implementation.${method.name}(${indent(passParams.join(', '), ' ')})
return ${indent(returnValue, ' ')}
} catch {
let __message = "\\(error.localizedDescription)"
fatalError("Swift errors can currently not be propagated to C++! See https://github.com/swiftlang/swift/issues/75290 (Error: \\(__message))")
${indent(body, ' ')}
} catch (let __error) {
let __exceptionPtr = ${indent(bridgedErrorType.parseFromSwiftToCpp('__error', 'swift'), ' ')}
return bridge.${resultBridge.funcName}(__exceptionPtr)
}
}
`.trim()
Expand Down
49 changes: 49 additions & 0 deletions packages/nitrogen/src/syntax/types/ResultWrappingType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import type { Language } from '../../getPlatformSpecs.js'
import { type SourceFile, type SourceImport } from '../SourceFile.js'
import { ErrorType } from './ErrorType.js'
import type { Type, TypeKind } from './Type.js'

export class ResultWrappingType implements Type {
readonly result: Type
readonly error: Type

constructor(result: Type) {
this.result = result
this.error = new ErrorType()
}

get canBePassedByReference(): boolean {
return this.result.canBePassedByReference
}

get kind(): TypeKind {
return 'result-wrapper'
}

getCode(language: Language): string {
switch (language) {
case 'c++':
return `Result<${this.result.getCode(language)}>`
case 'swift':
return this.result.getCode(language)
default:
throw new Error(
`Language ${language} is not yet supported for VariantType!`
)
}
}
getExtraFiles(): SourceFile[] {
return [...this.result.getExtraFiles(), ...this.error.getExtraFiles()]
}
getRequiredImports(): SourceImport[] {
return [
{
language: 'c++',
name: 'NitroModules/Result.hpp',
space: 'system',
},
...this.result.getRequiredImports(),
...this.error.getRequiredImports(),
]
}
}
1 change: 1 addition & 0 deletions packages/nitrogen/src/syntax/types/Type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type TypeKind =
| 'struct'
| 'tuple'
| 'variant'
| 'result-wrapper'
| 'void'

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ class HybridTestObjectKotlin: HybridTestObjectSwiftKotlinSpec() {
throw Error("This function will only work after sacrificing seven lambs!")
}

override fun funcThatThrowsBeforePromise(): Promise<Unit> {
throw Error("This function will only work after sacrificing eight lambs!")
}

override fun throwError(error: Throwable): Unit {
throw error
}
Expand Down
4 changes: 4 additions & 0 deletions packages/react-native-nitro-image/cpp/HybridTestObjectCpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ double HybridTestObjectCpp::funcThatThrows() {
throw std::runtime_error("This function will only work after sacrificing seven lambs!");
}

std::shared_ptr<Promise<void>> HybridTestObjectCpp::funcThatThrowsBeforePromise() {
throw std::runtime_error("This function will only work after sacrificing eight lambs!");
}

void HybridTestObjectCpp::throwError(const std::exception_ptr& error) {
std::rethrow_exception(error);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
std::shared_ptr<AnyMap> createMap() override;
std::shared_ptr<AnyMap> mapRoundtrip(const std::shared_ptr<AnyMap>& map) override;
double funcThatThrows() override;
std::shared_ptr<Promise<void>> funcThatThrowsBeforePromise() override;
void throwError(const std::exception_ptr& error) override;
std::string tryOptionalParams(double num, bool boo, const std::optional<std::string>& str) override;
std::string tryMiddleParam(double num, std::optional<bool> boo, const std::string& str) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ class HybridTestObjectSwift : HybridTestObjectSwiftKotlinSpec {
func callWithOptional(value: Double?, callback: @escaping ((_ maybe: Double?) -> Void)) throws -> Void {
callback(value)
}

func getValueFromJSCallbackAndWait(getValue: @escaping (() -> Promise<Double>)) throws -> Promise<Double> {
return .async {
let jsResult = try await getValue().await()
return jsResult
}
}

func getValueFromJsCallback(callback: @escaping (() -> Promise<String>), andThenCall: @escaping ((_ valueFromJs: String) -> Void)) throws -> Promise<Void> {
return .async {
let jsResult = try await callback().await()
Expand Down Expand Up @@ -125,14 +125,15 @@ class HybridTestObjectSwift : HybridTestObjectSwiftKotlinSpec {
}

func funcThatThrows() throws -> Double {
// TODO: Swift functions can not throw yet! Errors are not propagated up to C++.
// throw RuntimeError.error(withMessage: "This function will only work after sacrificing seven lambs!")
return 55
throw RuntimeError.error(withMessage: "This function will only work after sacrificing seven lambs!")
}

func funcThatThrowsBeforePromise() throws -> Promise<Void> {
throw RuntimeError.error(withMessage: "This function will only work after sacrificing eight lambs!")
}

func throwError(error: Error) throws -> Void {
// TODO: Swift functions can not throw yet! Errors are not propagated up to C++.
// throw error
throw error
}

func tryOptionalParams(num: Double, boo: Bool, str: String?) throws -> String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,21 @@ namespace margelo::nitro::image {
auto __result = method(_javaPart);
return __result;
}
std::shared_ptr<Promise<void>> JHybridTestObjectSwiftKotlinSpec::funcThatThrowsBeforePromise() {
static const auto method = _javaPart->getClass()->getMethod<jni::local_ref<JPromise::javaobject>()>("funcThatThrowsBeforePromise");
auto __result = method(_javaPart);
return [&]() {
auto __promise = Promise<void>::create();
__result->cthis()->addOnResolvedListener([=](const jni::alias_ref<jni::JObject>& __boxedResult) {
__promise->resolve();
});
__result->cthis()->addOnRejectedListener([=](const jni::alias_ref<jni::JThrowable>& __throwable) {
jni::JniException __jniError(__throwable);
__promise->reject(std::make_exception_ptr(__jniError));
});
return __promise;
}();
}
void JHybridTestObjectSwiftKotlinSpec::throwError(const std::exception_ptr& error) {
static const auto method = _javaPart->getClass()->getMethod<void(jni::alias_ref<jni::JThrowable> /* error */)>("throwError");
method(_javaPart, jni::getJavaExceptionForCppException(error));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ namespace margelo::nitro::image {
std::shared_ptr<AnyMap> createMap() override;
std::shared_ptr<AnyMap> mapRoundtrip(const std::shared_ptr<AnyMap>& map) override;
double funcThatThrows() override;
std::shared_ptr<Promise<void>> funcThatThrowsBeforePromise() override;
void throwError(const std::exception_ptr& error) override;
std::string tryOptionalParams(double num, bool boo, const std::optional<std::string>& str) override;
std::string tryMiddleParam(double num, std::optional<bool> boo, const std::string& str) override;
Expand Down
Loading

0 comments on commit 5b3fa6c

Please sign in to comment.