Skip to content

Conversation

@farfromrefug
Copy link
Contributor

This is what we discussed on Discord.
If a native obj-c method like this openReturningError(error?) is called without the error argument it will throw a JS Error if an error is "triggered"
The JS Error named "NSError" will have a localizedDescription as the message and code,domain,nativeException properties.

@edusperoni dont hesitate to comment on the actual code. I am no v8 expert and i was helped by AI.
I tested the code and it works.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances NSError handling in the NativeScript runtime by wrapping native NSError objects in JavaScript Error objects with proper stack traces and metadata. When Objective-C methods with NSError** parameters are called without passing an error reference and an error occurs, the runtime now throws a proper JavaScript Error instead of a generic NativeScriptException.

Key changes:

  • Modified error handling in Interop.mm to create JavaScript Error objects with attached NSError properties (code, domain, nativeException)
  • Added comprehensive test coverage for NSError wrapping behavior in both throw and non-throw scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
NativeScript/runtime/Interop.mm Replaced simple NativeScriptException throw with creation of JavaScript Error object that wraps NSError with proper stack trace and metadata
TestRunner/app/tests/ApiTests.js Added two new test cases to verify NSError wrapping behavior when called with and without error reference parameter

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1548 to +1549
ObjCDataWrapper* wrapper = new ObjCDataWrapper(error);
Local<Value> nativeWrapper = ArgConverter::CreateJsWrapper(context, wrapper, Local<Object>(), true);
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ObjCDataWrapper is created with skipGCRegistration=true in CreateJsWrapper, but there's no explicit cleanup if the Error object creation or subsequent Set operations fail. Consider using a smart pointer or ensuring the wrapper is properly cleaned up in the error path to prevent memory leaks.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant