From e160f543cac42f4bcdcf9307cda06eed578dbdf6 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Fri, 8 Nov 2024 04:24:12 -0800 Subject: [PATCH] Client error message Summary: See previous diff in stack for context, serialization can fail and when it does we just send an empty string payload to flipper which is v confusing. Instead we now do proper error handling A note about the use of try catch around nsjson serialization i know this is not typical but the previous approach was to use the validator function to see if data could be serialized. this was returning false before but the problem is we have idea what the error is. Unfortunatley despite taking an error parm the NSJSONSerialization dataWithJSONObject function throws an exception so im force to catch it to get a useful error message Differential Revision: D65605276 fbshipit-source-id: 506ecc48bd0692a622679a8f621fede7d626c122 --- .../Observer/UIDTreeObserverManager.m | 54 +++++++++++++++-- .../Serializers/UIDJSONSerializer.h | 4 +- .../Serializers/UIDJSONSerializer.mm | 60 ++++++++++++------- 3 files changed, 91 insertions(+), 27 deletions(-) diff --git a/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Observer/UIDTreeObserverManager.m b/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Observer/UIDTreeObserverManager.m index d46397aea1f..2d76a216987 100644 --- a/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Observer/UIDTreeObserverManager.m +++ b/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Observer/UIDTreeObserverManager.m @@ -207,7 +207,37 @@ - (void)sendInit { init.frameworkEventMetadata = [_context.frameworkEventManager eventsMetadata]; init.currentTraversalMode = _traversalMode; - [_context.connection send:[UIDInitEvent name] withRawParams:UID_toJSON(init)]; + NSError* error = nil; + if (error) { + [self sendClientErrorMessage:error step:@"init"]; + return; + } + [_context.connection send:[UIDInitEvent name] + withRawParams:UID_toJSON(init, &error)]; +} + +- (void)sendClientErrorMessage:(NSError*)error step:(NSString*)step { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wnullable-to-nonnull-conversion" + + NSDictionary* clientError = @{ + @"errorMessage" : error.localizedDescription, + @"errorType" : error.localizedFailureReason, + @"step" : step, + }; + + NSError* errorMessageSerializeError = nil; + NSString* clientErrorEvent = + UID_FoundationtoJSON(clientError, &errorMessageSerializeError); + + if (errorMessageSerializeError) { + // uh oh... + return; + } + + [_context.connection send:@"traversalError" withRawParams:clientErrorEvent]; + +#pragma clang diagnostic pop } - (void)sendMetadataUpdate { @@ -220,7 +250,12 @@ - (void)sendMetadataUpdate { UIDMetadataUpdateEvent* metadataUpdateEvent = [UIDMetadataUpdateEvent new]; metadataUpdateEvent.attributeMetadata = pendingMetadata; - id JSON = UID_toJSON(metadataUpdateEvent); + NSError* error = nil; + id JSON = UID_toJSON(metadataUpdateEvent, &error); + if (error) { + [self sendClientErrorMessage:error step:@"metadataUpdate"]; + return; + } [_context.connection send:[UIDMetadataUpdateEvent name] withRawParams:JSON]; } @@ -252,7 +287,14 @@ - (void)digest:(nonnull id)update { uint64_t t3 = UIDPerformanceNow(); - NSString* JSON = UID_FoundationtoJSON(intermediate); + NSError* error = nil; + + NSString* JSON = UID_FoundationtoJSON(intermediate, &error); + if (error) { + [self sendClientErrorMessage:error step:@"serialization"]; + return; + } + uint64_t t4 = UIDPerformanceNow(); NSUInteger payloadSize = JSON.length; @@ -280,8 +322,12 @@ - (void)digest:(nonnull id)update { perfStats.payloadSize = payloadSize; perfStats.dynamicMeasures = UIDPerformanceGet(); + NSString* perfStatsMessage = UID_toJSON(perfStats, &error); + if (error) { + [self sendClientErrorMessage:error step:@"perfStats"]; + } [self->_context.connection send:[UIDPerfStatsEvent name] - withRawParams:UID_toJSON(perfStats)]; + withRawParams:perfStatsMessage]; }); } diff --git a/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.h b/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.h index 9dd4f302076..29022ba55fa 100644 --- a/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.h +++ b/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.h @@ -17,8 +17,8 @@ #ifdef __cplusplus extern "C" { #endif -extern NSString* UID_toJSON(id obj); -extern NSString* UID_FoundationtoJSON(id obj); +extern NSString* UID_toJSON(id obj, NSError** error); +extern NSString* UID_FoundationtoJSON(id obj, NSError** error); extern id UID_toFoundation(id obj); #ifdef __cplusplus } diff --git a/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.mm b/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.mm index 7ef66c70b7d..7b62db47d74 100644 --- a/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.mm +++ b/iOS/Plugins/FlipperKitUIDebuggerPlugin/FlipperKitUIDebuggerPlugin/Serializers/UIDJSONSerializer.mm @@ -30,37 +30,55 @@ id UID_toFoundation(id object) { return [object toFoundation]; } +NSString* UID_FoundationtoJSON(id object, NSError** error) { + uint64_t t0 = UIDPerformanceNow(); -NSString* UID_FoundationtoJSON(id object) { - NSError* error = NULL; - if (![NSJSONSerialization isValidJSONObject:object]) { - return @""; - } + // NSJSONSerialization dataWithJSONObject throws and exception if data not + // serializable despite taking an error param hence the try catch + @try { + uint64_t t1 = UIDPerformanceNow(); - uint64_t t0 = UIDPerformanceNow(); + NSData* data = [NSJSONSerialization dataWithJSONObject:object + options:0 + error:error]; + + if (*error) { + return nil; + } - NSData* data = [NSJSONSerialization dataWithJSONObject:object - options:0 - error:&error]; - uint64_t t1 = UIDPerformanceNow(); + UIDPerformanceAggregate( + @"serialisationBinaryMS", + UIDMonotonicTimeConvertMachUnitsToMS(t1 - t0)); - UIDPerformanceAggregate( - @"serialisationBinaryMS", UIDMonotonicTimeConvertMachUnitsToMS(t1 - t0)); + NSString* JSON = [[NSString alloc] initWithData:data + encoding:NSUTF8StringEncoding]; - NSString* JSON = [[NSString alloc] initWithData:data - encoding:NSUTF8StringEncoding]; + uint64_t t2 = UIDPerformanceNow(); - uint64_t t2 = UIDPerformanceNow(); + UIDPerformanceAggregate( + @"serialisationEncodingMS", + UIDMonotonicTimeConvertMachUnitsToMS(t2 - t1)); - UIDPerformanceAggregate( - @"serialisationEncodingMS", - UIDMonotonicTimeConvertMachUnitsToMS(t2 - t1)); + return JSON; - return JSON; + } @catch (NSException* exception) { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wnullable-to-nonnull-conversion" + *error = [NSError errorWithDomain:@"com.facebook.flipper.uidebugger" + code:100 + userInfo:@{ + NSLocalizedDescriptionKey : exception.reason, + NSLocalizedFailureReasonErrorKey : exception.name + }]; + + return nil; + +#pragma clang diagnostic pop + } } -NSString* UID_toJSON(id object) { - return UID_FoundationtoJSON(UID_toFoundation(object)); +NSString* UID_toJSON(id object, NSError** error) { + return UID_FoundationtoJSON(UID_toFoundation(object), error); } #ifdef __cplusplus }