Publishing fails without error on previously running code

[mmubasher]

Bug Report

Current behavior

Video publishing from Ionic based Cordova app has stopped. It worked fine some time ago. No errors in console found.

Example Project

Here is an example project

Steps to reproduce

Simply clone the above mentioned project which has all the setup information in README document

What is the current bug behavior?

without any errors video publishing does not starts on android device

What is the expected correct behavior?

Video should start publishing as it worked before

Relevant logs and/or screenshots
Following is the current log. Also, there is no opentok related network log of this cordova application's webview

Network log

Console log

Ionic Native: deviceready event fired after 732 ms
opentok.js:519 domId exists but properties width or height is not specified
opentok.js:521  width: 212 and height: 177 for domId publisher, and top: 56, left: 200
opentok.js:317 JS Lib: creating publisher -  Object__proto__: Object
opentok.js:248 JS: Objects being updated in TBUpdateObjects
opentok.js:253 JS: Object updated
opentok.js:255 JS sessionId: TBPublisher
opentok.js:519 domId exists but properties width or height is not specified
opentok.js:521  width: 212 and height: 177 for domId publisher, and top: 56, left: 200
opentok.js:317 JS Lib: creating publisher -  {}__proto__: Object
opentok.js:248 JS: Objects being updated in TBUpdateObjects
opentok.js:253 JS: Object updated
opentok.js:255 JS sessionId: TBPublisher

Environment information

$ ionic info

Ionic:
   Ionic CLI                     : 5.0.0 (C:\Users\mmubasher\AppData\Roaming\npm\node_modules\ionic)
   Ionic Framework               : @ionic/angular 4.0.0-beta.16
   @angular-devkit/build-angular : 0.10.7
   @angular-devkit/schematics    : 7.0.7
   @angular/cli                  : 7.0.7
   @ionic/angular-toolkit        : 1.2.0

Cordova:
   Cordova CLI       : 8.1.2 ([email protected])
   Cordova Platforms : android 7.1.4
   Cordova Plugins   : cordova-plugin-ionic-keyboard 2.1.3, cordova-plugin-ionic-webview 2.5.1, (and 5 other plugins)

Utility:
   cordova-res : not installed
   native-run  : 0.2.7

System:
   Android SDK Tools : 26.1.1 (D:\programs\android-sdk)
   NodeJS            : v10.12.0 (C:\Program Files\nodejs\node.exe)
   npm               : 6.4.1
   OS                : Windows 10

[wolfenrain]

We were also having this problem when I was trying to update our own fork of this plugin with the current master.

This is happening because of the fix for #107.

This fix does not wait for the sessionConnected event. Instead it calls the callback instantly after the Session.connect is called. And according to the docs:

sessionConnected: Dispatched locally by the Session object when the connection is established. However, you can pass a completion handler function in as the second parameter of the connect() and use this function instead of a listener for the sessionConnected event.

The completion handler should be called upon connecting, aka on sessionConnected. This was the case before the fix was implemented.

I think we should partly revert the change into something like this:

OTSession.coffee:

class TBSession
  connect: (@token, connectCompletionCallback) ->
    if( typeof(connectCompletionCallback) != "function" and connectCompletionCallback? )
      TB.showError( "Session.connect() takes a token and an optional completionHandler" )
      return
    if (connectCompletionCallback?)
      errorCallback = (error) -> connectCompletionCallback(error)
      @on('sessionConnected', connectCompletionCallback)
    Cordova.exec(@eventReceived, TBError, OTPlugin, "addEvent", ["sessionEvents"] )
    Cordova.exec(TBSuccess, errorCallback, OTPlugin, "connect", [@token] )
    return
  ...

OpenTokAndroidPlugin.java:

    @Override
    public boolean execute(String action, JSONArray args, CallbackContext callbackContext) throws JSONException {
        ...
        } else if (action.equals("connect")) {
            Log.i(TAG, "connect command called");
            mSession.connect(args.getString(0));
        } else if (action.equals("disconnect")) {
        ...
    }

OpenTokPlugin.m:

#pragma mark Session Methods
- (void)connect:(CDVInvokedUrlCommand *)command{
    NSLog(@"iOS Connecting to Session");

    // Get Parameters
    OTError *error = nil;
    NSString* tbToken = [command.arguments objectAtIndex:0];
    [_session connectWithToken:tbToken error:&error];
    CDVPluginResult* pluginResult;
    if (error) {
        NSNumber* code = [NSNumber numberWithInt:[error code]];
        NSMutableDictionary* err = [[NSMutableDictionary alloc] init];
        [err setObject:error.localizedDescription forKey:@"message"];
        [err setObject:code forKey:@"code"];
        pluginResult = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsDictionary:err];
        [self.commandDelegate sendPluginResult:pluginResult callbackId:command.callbackId];
    }
}

That way we will still have the error handling as was mentioned in
#107. And when no errors happens our callback will be triggered when the session is connected, as it should be according to the docs.

What do you think @msach22?