Adding fix for the missing nil checks. #356
Conversation
| resolver:(RCTPromiseResolveBlock)resolve | ||
| rejecter:(RCTPromiseRejectBlock)reject) { | ||
| // Validate content dictionary | ||
| if (!content || ![content isKindOfClass:[NSDictionary class]]) { |
There was a problem hiding this comment.
if content type changes in ios and android, we will have to update here as well. What if we move validation in android and ios sdks and make sure proper error is returned via RN as well?
| NSString *contentType = content[@"type"]; | ||
|
|
||
| // Add nil check before calling methods | ||
| if (!contentType || ![contentType isKindOfClass:[NSString class]]) { |
There was a problem hiding this comment.
same comment as above and for others validations
| } | ||
| }; | ||
|
|
||
| const ensureString = (value: string, fallback = '') => |
There was a problem hiding this comment.
we can bypass this validation by passing wrong value as fallback param, for example, ensureString(1, 1) would return 1, even though its not a string. I think we should just return error that if value is not string, return error
| const ensureString = (value: string, fallback = '') => | ||
| typeof value === 'string' ? value : fallback; | ||
|
|
||
| const ensureNumber = (value: number, fallback = 0) => |
There was a problem hiding this comment.
same comment as above and in other methods
| if (intercomContent) { | ||
| [Intercom presentContent:intercomContent]; | ||
| resolve(@(YES)); | ||
| } else { | ||
| reject(@"CONTENT_CREATION_FAILED", @"Failed to create Intercom content", nil); | ||
| } |
There was a problem hiding this comment.
This is valid without this we would have a never resolved promise
|
I fear we are being overly defensive in this PR:
It would however, make sense to have a single validation wrapper at the API boundary that reports back to our error reporting platform, that would give us a better insight into what’s actually failing in production. |
adding defensive checks for the missing nil checks on the react native code base.