From ea9e8ee2c61a8ca1d8b7ac1f9fa203205ee18d35 Mon Sep 17 00:00:00 2001 From: nstudio Date: Fri, 22 May 2026 09:31:23 -0700 Subject: [PATCH] fix(ios): NSError** runtime fallback and enriched arg-count diagnostics Adds defensive coverage and improved error visibility around NSError** out-parameter handling on modern iOS SDKs. Runtime: - Metadata.h: hasErrorOutParameter() now falls back to inspecting the last binary type encoding when the MethodHasErrorOutParameter flag is clear. Mirrors what ArgConverter::IsErrorOutParameter already does on the callback path. Lets apps shipping older/buggy metadata recover correct auto-NSError** handling without regenerating metadata. Diagnostics: - ArgConverter.mm: argument-count error message had its Actual/Expected labels swapped and no call-site context. Fixed the swap and added method kind, JS name, class name, full Objective-C selector, and a conditional hint when the method has an NSError** out-parameter that callers may omit. - ObjectManager.mm: ReleaseNativeCounterpartCallback error message now names the function it is about (__releaseNativeCounterpart). Documentation: - MetaFactory.cpp: explanatory comment block documenting why the existing NullableType unwrap (already on main via #367) is needed for "NSError * _Nullable * _Nullable" selectors on modern SDKs. --- NativeScript/runtime/ArgConverter.mm | 19 ++++++++++-- NativeScript/runtime/Metadata.h | 33 ++++++++++++++++++++- NativeScript/runtime/ObjectManager.mm | 3 +- metadata-generator/src/Meta/MetaFactory.cpp | 11 +++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index bd6956c6..d7a2173c 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -104,9 +104,24 @@ if ((!meta->hasErrorOutParameter() && args.Length() != argsCount) || (meta->hasErrorOutParameter() && args.Length() != argsCount && args.Length() != argsCount - 1)) { + // NOTE: The message used to read "Actual arguments count: . Expected: .", + // which had the two numbers swapped. This made the log impossible to act on without reading + // the runtime source. We now spell it out (with swap corrected) and include the call site. + const char* jsNameStr = meta ? meta->jsName() : ""; + const char* selectorStr = meta ? meta->selectorAsString() : ""; + const char* classNameStr = klass ? class_getName(klass) : ""; + const char* methodKindStr = instanceMethod ? "instance" : "static"; + std::ostringstream errorStream; - errorStream << "Actual arguments count: \"" << argsCount << ". Expected: \"" << args.Length() - << "\"."; + errorStream << "Actual arguments count: \"" << args.Length() << "\". Expected: \"" << argsCount + << "\""; + if (meta && meta->hasErrorOutParameter()) { + errorStream << " (or \"" << (argsCount - 1) + << "\" if omitting the trailing NSError** out-parameter)"; + } + errorStream << " for " << methodKindStr << " method \"" << jsNameStr << "\" on class \"" + << classNameStr << "\" (selector: -[" << classNameStr << " " << selectorStr + << "])."; std::string errorMessage = errorStream.str(); throw NativeScriptException(errorMessage); } diff --git a/NativeScript/runtime/Metadata.h b/NativeScript/runtime/Metadata.h index 0d0dc701..51e37acc 100644 --- a/NativeScript/runtime/Metadata.h +++ b/NativeScript/runtime/Metadata.h @@ -1,6 +1,7 @@ #ifndef Metadata_h #define Metadata_h +#include #include #include #include @@ -748,7 +749,37 @@ struct MethodMeta : MemberMeta { } inline bool hasErrorOutParameter() const { - return this->flag(MetaFlags::MethodHasErrorOutParameter); + if (this->flag(MetaFlags::MethodHasErrorOutParameter)) { + return true; + } + + // Fallback: The MethodHasErrorOutParameter flag is set by the metadata + // generator via a type-tree walk that historically missed methods whose + // last parameter uses `_Nullable` nullability (e.g. + // `NSError * _Nullable * _Nullable` on modern iOS SDK headers). + // The *binary* encoding does preserve the underlying shape (NullableType + // is unwrapped during serialization), so we can recover the missing + // signal by inspecting the last parameter encoding directly. This keeps + // apps built against older/buggy metadata working with the auto-error + // handling in Interop::CallFunctionInternal and ArgConverter::Invoke. + const TypeEncodingsList* enc = this->encodings(); + if (enc == nullptr || enc->count < 2) { + return false; + } + // TypeEncodingsList layout: [returnType, param1, param2, ..., lastParam] + const TypeEncoding* cur = enc->first(); + for (int i = 0; i < enc->count - 1; i++) { + cur = cur->next(); + } + if (cur == nullptr || cur->type != BinaryTypeEncodingType::PointerEncoding) { + return false; + } + const TypeEncoding* innerEnc = cur->details.pointer.getInnerType(); + if (innerEnc == nullptr || innerEnc->type != BinaryTypeEncodingType::InterfaceDeclarationReference) { + return false; + } + const char* innerName = innerEnc->details.interfaceDeclarationReference.name.valuePtr(); + return innerName != nullptr && strcmp(innerName, "NSError") == 0; } inline bool isInitializer() const { diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 75df80d7..f6e8650a 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -176,7 +176,8 @@ if (info.Length() != 1) { std::ostringstream errorStream; - errorStream << "Actual arguments count: \"" << info.Length() << "\". Expected: \"1\"."; + errorStream << "Actual arguments count: \"" << info.Length() << "\". Expected: \"1\"." + << " (function: __releaseNativeCounterpart(nativeObject))"; std::string errorMessage = errorStream.str(); Local error = Exception::Error(tns::ToV8String(isolate, errorMessage)); isolate->ThrowException(error); diff --git a/metadata-generator/src/Meta/MetaFactory.cpp b/metadata-generator/src/Meta/MetaFactory.cpp index 326b4a7a..25ea2c96 100644 --- a/metadata-generator/src/Meta/MetaFactory.cpp +++ b/metadata-generator/src/Meta/MetaFactory.cpp @@ -490,6 +490,17 @@ void MetaFactory::createFromMethod(const clang::ObjCMethodDecl& method, isNullTerminatedVariadic); // set MethodHasErrorOutParameter flag + // + // Modern iOS SDK headers annotate out-error parameters as + // `NSError * _Nullable * _Nullable`, which TypeFactory wraps in + // `NullableType`/`NonNullableType` nodes (see + // `TypeFactory::createFromAttributedType`). The original implementation only + // checked for a raw `PointerType(InterfaceType("NSError"))` and therefore + // missed the vast majority of NSError**-returning Objective-C selectors + // (createDirectoryAtPath:...:error:, removeItemAtPath:error:, etc.). Unwrap + // the nullability wrapper at both the outer pointer and inner interface + // levels so the flag is set correctly whether or not the SDK annotates + // nullability. if (method.parameters().size() > 0) { clang::ParmVarDecl* lastParameter = method.parameters()[method.parameters().size() - 1];