amazon-archives / aws-sdk-react-native

AWS SDK for React Native (developer preview)
Apache License 2.0
631 stars 68 forks source link

Core project bug - Cognito RCTJSONStringify errors (NSTaggedDate, __NSMallocBlock__) #3

Open willks opened 7 years ago

willks commented 7 years ago

Hi, I've banged my head for a few hours on this issue when integrating with my project - and it seems that the reference example project is also broken. The issue is this block in the example/index.js

async function getCredAndID(){
      try{
        var variable = await AWSCognitoCredentials.getCredentialsAsync();
        that.setState({AccessKey:variable["AccessKey"],SecretKey:variable["SecretKey"],
        SessionKey:variable["SessionKey"],Expiration:variable["Expiration"].toString()});
        variable = await AWSCognitoCredentials.getIdentityIDAsync();
        that.setState({identityID:variable.identityid});
      }catch(e){
        console.log("Error: " + e)
        return;
      }
    }

Specifically, the line:

var variable = await AWSCognitoCredentials.getCredentialsAsync();

Always results in an error as follows:

screen shot 2016-09-01 at 8 19 59 pm

And the error in plain text:

2016-09-01 20:17:29.399 example[95700:996058] *** -[NSLock lock]: deadlock (<NSLock: 0x7f87ab57c3b0> '(null)')
2016-09-01 20:17:29.399 example[95700:996058] *** Break on _NSLockError() to debug.
2016-09-01 20:17:29.400 [error][tid:com.facebook.react.WebSocketExecutor][RCTUtils.m:80] RCTJSONStringify() encountered the following error: Invalid type in JSON write (__NSMallocBlock__)

Looks like the bundled library is trying to send an event with a "block" payload. Then, you have another bug, which I found only when I modified the AWSCognitoCredentials.m - you can't do this in react native:

RCT_EXPORT_METHOD(getCredentialsAsync:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject){
    [[credentialProvider credentials] continueWithBlock:^id(AWSTask *task) {
        if (task.exception){
            dispatch_async(dispatch_get_main_queue(), ^{
                @throw [NSException exceptionWithName:task.exception.name reason:task.exception.reason userInfo:task.exception.userInfo];
            });
        }
        if (task.error) {
            reject([NSString stringWithFormat:@"%ld",task.error.code],task.error.description,task.error);
        }
        else {
            AWSCredentials *cred = (AWSCredentials*) task.result;
            NSDictionary *dict = @{@"AccessKey":cred.accessKey,@"SecretKey":cred.secretKey,@"SessionKey":cred.sessionKey,@"Expiration":cred.expiration};
            resolve(dict);
        }
        return nil;
    }];
}

You are sending an NSTaggedDate (NSDate) here: cred.expiration, which is not serializable by the RCT Bridge! I never got it working, (the example) - otherwise I would have submitted a pull request but I'm a green ObjC dev.

I really hope that it's just me and that the underlying framework is not buggy - as I really need this project :)

Thanks

willks commented 7 years ago

Responding to myself here. In the class "AWSCognitoCredentials.m" If I comment out the body of the method "sendMessage"

and do this for "getCredentialsAsync", then I got it working!

RCT_EXPORT_METHOD(getCredentialsAsync:(RCTPromiseResolveBlock) resolve rejecter:(RCTPromiseRejectBlock) reject) {
  [[credentialProvider credentials] continueWithBlock:^id(AWSTask *task) {
      if (task.exception) {
        dispatch_async(dispatch_get_main_queue(), ^{
            @throw [NSException exceptionWithName:task.exception.name reason:task.exception.reason userInfo:task.exception.userInfo];
        });
      }
      if (task.error) {
        reject([NSString stringWithFormat:@"%ld", task.error.code], task.error.description, task.error);
      } else {
        AWSCredentials *cred = (AWSCredentials *) task.result;
        // We want a javascript friendly epoch (in ms)
        NSNumber *milliseconds = [NSNumber numberWithLong:cred.expiration.timeIntervalSince1970 * 1000.0];
            NSDictionary * dict = @{@"AccessKey": cred.accessKey, @"SecretKey": cred.secretKey, @"SessionKey": cred.sessionKey, @"Expiration": milliseconds};
        resolve(dict);
      }
      return nil;
  }];
}

I'm doing this for now, it gets me unblocked until a better fix comes my way!

appwiz commented 7 years ago

Thanks for the issue report and the fix. I'm looking into it.

willks commented 7 years ago

Actually, this may not be a total fix at all, the events had to be disabled. With my hacks, I can't get "isAuthenticated" to ever return true, so there may be deeper issues that I'm not totally understanding.

willks commented 7 years ago

I forked the project for now, got my login/logout and unauthenticated access going through AWS API Gateway. Tested for hours now with multiple login/logout scenarios and going from unauthenticated to authenticated access. I'm sure my fork will disgust even the greenest of developers, but for now it serves my basic purpose - hacked up just for facebook.

Thanks

willks commented 7 years ago

Any updates on this issue?

appwiz commented 7 years ago

Hi @willks, sorry, no update yet. Still looking into this issue.

ElChapitan commented 7 years ago

I'm actually having this issue as well. I'm fairly positive that I only started receiving this error after upgrading to Xcode 8.0 (I was previously on 7.2.1). Not sure if that will help anyone track down the bugs.

@willks which version of Xcode did you have running?

ElChapitan commented 7 years ago

After poking around a bit more with the debugger, I have a better idea of where the problem is thanks to the extra info from @willks. I think the problem is in AWSCognitoCredentials.m and specifically in the sendMessage function:

-(void)sendMessage:(NSMutableDictionary*)info toChannel:(NSString*)channel withCallback:(RCTResponseSenderBlock)callback{
    if ([channel isEqualToString:@"LoginsRequestedEvent"]){
        [lock lock];
        [info setValue:callback forKey:@"ReturnInfo"];
    }
    [self.bridge.eventDispatcher
     sendAppEventWithName:channel
     body:[info copy]
     ];
}

After a whole lot of debugging, it seems like the culprit is the call to [info setValue:callback forKey:@"ReturnInfo"] which is supposed to be a callback for the Javascript to return values back to the Objective-C side of the game. From what I can tell, RCTJSONStringify() seems to be blowing up trying to convert that callback into JSON, but then still returns values into the Javascript, when it gets to the following chunk of code in the javascript, it blows up again with a NPE.

      listener.addListener("LoginsRequestedEvent", async event => {
        event.ReturnInfo([this.getLogins()]);
      });

I might have more time to poke around again later and figure more out, but hopefully this is helpful to someone.

Current setup:

XCode 8.0 "react-native": "0.31.0"

pentateu commented 7 years ago

Same issue. I have found the same as @ElChapitan

+1

pentateu commented 7 years ago

hi @appwiz , Is this a xcode 8.0 only issue? I have not tested with XCode 7.x , so I'm wondering if I should go back to XCODE 7 or not.

willks commented 7 years ago

@ElChapitan That's exactly the issue - the attempted serialisation of the block. However after I commented that out (as I didn't need it) - the serialisation of the NSDate also caught me out. I had to convert the millis to and use the timestamp. Could also work as an ISO date string though. My crappy fork got me over the line, but I notice that my version does too many calls to cognito initially - it's just a stop-gap until the bug is sorted.

@pentateu I had the issue with XCode 7.3 but I'm on 8 now as I have to get my app running on iOS 10. It's all working now, using RN 0.33, but I would be happier if I didn't have to hack around this issue. I doubt this is an XCode version issue as RN doesn't support serialisation for just any type. The types supported are here: https://facebook.github.io/react-native/docs/native-modules-ios.html#argument-types

willks commented 7 years ago

@appwiz @ElChapitan @pentateu

Is this project still active? Is anyone using it for anything serious? I, unfortunately, had to stop it. I have no time to contribute freely to a company that charges us for their services.

mnichols commented 7 years ago

This bug is due to the destructuring bug found here. Change it to

listener.addListener("LoginsRequestedEvent", async ({callbackId}) => {

And the event being raised as a dictionary (from native) will properly be destructured after conversion to a JSON object.

willks commented 7 years ago

This worked! I got it half working before, but this is much better. I changed it on my local fork and so far, it's good! Great find!