Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ios): manually evaluate all error properties #11576

Merged
merged 8 commits into from
Apr 8, 2020
Merged
6 changes: 2 additions & 4 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/KrollBridge.m
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,8 @@ - (void)evalFileOnThread:(NSString *)path context:(KrollContext *)context_
}
}
if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
evaluationError = YES;
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
Copy link
Contributor

Choose a reason for hiding this comment

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

@janvennemann defaultExceptionHandler is a method not property. I think it should be used as [TiExceptionHandler defaultExceptionHandler]. Same applies for other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, a property will only create a variable and getter/setters for you and the compiler will automatically call the getter method when you access the property. By using TiExceptionHandler.defaultExceptionHandler here I just instruct the compiler to automatically call the method defaultExceptionHandler, so it shouldn't make a difference.

}

JSStringRelease(jsCode);
Expand Down Expand Up @@ -692,8 +691,7 @@ - (KrollWrapper *)loadCommonJSModule:(NSString *)code withSourceURL:(NSURL *)sou
[eval release];

if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
Copy link
Contributor

Choose a reason for hiding this comment

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

@janvennemann I think two methods you introduced in TiExceptionHandler are not required. You can get TiScriptError by using the new method in TiUtils. Something like below -

[TiExceptionHandler.defaultExceptionHandler reportScriptError:[TiUtils scriptErrorFromValueRef: exception inContext: context.context]];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, they are not required. However, i added them purely for convenience reasons since we have different source types that we need to create TiScriptError from. I didn't like repeatedly having to use [TiUtils scriptErrorFromValueRef: exception inContext: context.context]] all over the place so i moved that into the two new methods.

IMHO [TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context]; looks nicer and is easier to read than having a nested method. Another benefit is that we only need to adjust the TiUtils usage in one place should we ever need to change this again.

return nil;
}
/*
Expand Down
3 changes: 1 addition & 2 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/ObjcProxy.m
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,7 @@ - (void)_fireEventToListener:(NSString *)type withObject:(id)obj listener:(JSVal
// handle an uncaught exception
JSValue *exception = listener.context.exception;
if (exception != nil) {
NSDictionary *exceptionDict = [self JSValueToNative:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:exceptionDict]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inJSContext:listener.context];
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/TiBindingEvent.m
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,7 @@ void TiBindingEventProcess(TiBindingRunLoop runloop, void *payload)
JSValueRef exception = NULL;
JSObjectCallAsFunction(context, (JSObjectRef)currentCallback, (JSObjectRef)eventTargetRef, 1, (JSValueRef *)&eventObjectRef, &exception);
if (exception != NULL) {
id excm = TiBindingTiValueToNSObject(context, exception);
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
}

// Note cancel bubble
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
*/

#import <Foundation/Foundation.h>
#import <JavaScriptCore/JSValueRef.h>

@class KrollContext;
@class JSValue;
@class JSContext;

#pragma mark - TiScriptError

Expand Down Expand Up @@ -107,4 +112,8 @@

- (void)reportScriptError:(TiScriptError *)error;

- (void)reportScriptError:(JSValueRef)errorRef inKrollContext:(KrollContext *)krollContext;

- (void)reportScriptError:(JSValue *)error inJSContext:(JSContext *)context;

@end
12 changes: 12 additions & 0 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/TiExceptionHandler.m
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

#import "TiExceptionHandler.h"
#import "APSAnalytics.h"
#import "KrollContext.h"
#import "TiApp.h"
#import "TiBase.h"

#include <JavaScriptCore/JavaScriptCore.h>
#include <execinfo.h>
#include <signal.h>

Expand Down Expand Up @@ -80,6 +82,16 @@ - (void)reportScriptError:(TiScriptError *)scriptError
[currentDelegate handleScriptError:scriptError];
}

- (void)reportScriptError:(JSValueRef)errorRef inKrollContext:(KrollContext *)krollContext
{
[self reportScriptError:[TiUtils scriptErrorFromValueRef:errorRef inContext:krollContext.context]];
}

- (void)reportScriptError:(JSValue *)error inJSContext:(JSContext *)context
{
[self reportScriptError:[TiUtils scriptErrorFromValueRef:error.JSValueRef inContext:context.JSGlobalContextRef]];
}

- (void)showScriptError:(TiScriptError *)error
{
NSArray<NSString *> *exceptionStackTrace = [error valueForKey:@"nativeStack"];
Expand Down
2 changes: 2 additions & 0 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,8 @@ typedef enum {

+ (TiScriptError *)scriptErrorValue:(id)value;

+ (TiScriptError *)scriptErrorFromValueRef:(JSValueRef)valueRef inContext:(JSGlobalContextRef)contextRef;

+ (NSTextAlignment)textAlignmentValue:(id)alignment;

+ (NSString *)jsonStringify:(id)value;
Expand Down
43 changes: 43 additions & 0 deletions iphone/TitaniumKit/TitaniumKit/Sources/API/TiUtils.m
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,49 @@ + (WebFont *)fontValue:(id)value
return [self fontValue:value def:[WebFont defaultFont]];
}

+ (TiScriptError *)scriptErrorFromValueRef:(JSValueRef)valueRef inContext:(JSGlobalContextRef)contextRef
{
JSContext *context = [JSContext contextWithJSGlobalContextRef:contextRef];
JSValue *error = [JSValue valueWithJSValueRef:valueRef inContext:context];
NSMutableDictionary *errorDict = [NSMutableDictionary new];

if ([error hasProperty:@"constructor"]) {
[errorDict setObject:[error[@"constructor"][@"name"] toString] forKey:@"type"];
}

// error message
if ([error hasProperty:@"message"]) {
[errorDict setObject:[error[@"message"] toString] forKey:@"message"];
}
if ([error hasProperty:@"nativeReason"]) {
[errorDict setObject:[error[@"nativeReason"] toString] forKey:@"nativeReason"];
}

// error location
if ([error hasProperty:@"sourceURL"]) {
[errorDict setObject:[error[@"sourceURL"] toString] forKey:@"sourceURL"];
}
if ([error hasProperty:@"line"]) {
[errorDict setObject:[error[@"line"] toNumber] forKey:@"line"];
}
if ([error hasProperty:@"column"]) {
[errorDict setObject:[error[@"column"] toNumber] forKey:@"column"];
}

// stack trace
if ([error hasProperty:@"backtrace"]) {
[errorDict setObject:[error[@"backtrace"] toString] forKey:@"backtrace"];
}
if ([error hasProperty:@"stack"]) {
[errorDict setObject:[error[@"stack"] toString] forKey:@"stack"];
}
if ([error hasProperty:@"nativeStack"]) {
[errorDict setObject:[error[@"nativeStack"] toString] forKey:@"nativeStack"];
}

return [[[TiScriptError alloc] initWithDictionary:errorDict] autorelease];
}

+ (TiScriptError *)scriptErrorValue:(id)value;
{
if ((value == nil) || (value == [NSNull null])) {
Expand Down
3 changes: 1 addition & 2 deletions iphone/TitaniumKit/TitaniumKit/Sources/Kroll/KrollCallback.m
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ - (id)call:(NSArray *)args thisObject:(id)thisObject_
JSValueRef exception = NULL;
JSValueRef retVal = JSObjectCallAsFunction(jsContext, function, tp, [args count], _args, &exception);
if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
}
if (top != NULL) {
JSValueUnprotect(jsContext, tp);
Expand Down
6 changes: 2 additions & 4 deletions iphone/TitaniumKit/TitaniumKit/Sources/Kroll/KrollContext.m
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,7 @@ - (void)invoke:(KrollContext *)context
[self jsInvokeInContext:context exception:&exception];

if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
pthread_mutex_unlock(&KrollEntryLock);
}
pthread_mutex_unlock(&KrollEntryLock);
Expand All @@ -549,8 +548,7 @@ - (id)invokeWithResult:(KrollContext *)context
JSValueRef result = [self jsInvokeInContext:context exception:&exception];

if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
pthread_mutex_unlock(&KrollEntryLock);
}
pthread_mutex_unlock(&KrollEntryLock);
Expand Down
6 changes: 2 additions & 4 deletions iphone/TitaniumKit/TitaniumKit/Sources/Kroll/KrollObject.m
Original file line number Diff line number Diff line change
Expand Up @@ -1043,8 +1043,7 @@ - (void)invokeCallbackForKey:(NSString *)key withObject:(NSDictionary *)eventDat
JSValueRef jsEventData = ConvertIdTiValue(context, eventData);
JSValueRef result = JSObjectCallAsFunction(jsContext, jsCallback, [_thisObject jsobject], 1, &jsEventData, &exception);
if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
}

if (block != nil) {
Expand Down Expand Up @@ -1279,8 +1278,7 @@ - (void)triggerEvent:(NSString *)eventName withObject:(NSDictionary *)eventData
JSValueRef exception = NULL;
JSObjectCallAsFunction(jsContext, (JSObjectRef)currentCallback, [thisObject jsobject], 1, &jsEventData, &exception);
if (exception != NULL) {
id excm = [KrollObject toID:context value:exception];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:excm]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inKrollContext:context];
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ - (void)timerFired:(NSTimer *_Nonnull)timer
JSContext *context = self.callback.context;
JSValue *exception = context.exception;
if (exception != nil) {
NSDictionary *exceptionDict = TiBindingTiValueToNSObject(context.JSGlobalContextRef, exception.JSValueRef);
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:exceptionDict]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:exception inJSContext:context];
}
}

Expand Down
3 changes: 1 addition & 2 deletions iphone/TitaniumKit/TitaniumKit/Sources/Kroll/KrollWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ - (JSValueRef)executeWithArguments:(NSArray<id> *)arguments
JSValueRef functionResult = JSObjectCallAsFunction(context, value, NULL, 1, callArgs, &callException);

if (callException != NULL) {
id exceptionPayload = [KrollObject toID:self.bridge.krollContext value:callException];
[[TiExceptionHandler defaultExceptionHandler] reportScriptError:[TiUtils scriptErrorValue:exceptionPayload]];
[TiExceptionHandler.defaultExceptionHandler reportScriptError:callException inKrollContext:context];
}

return functionResult;
Expand Down