Threading issues, force unwrap crashes and documentation

[simonmcl]

Crashes

I've come across 2 crashes related to this SDK or torus dependencies using force unwrapping. While experimenting It crashed in:

• TorusUtils.swift line 48
  • trying to force unwrap pub_key_X and pub_key_Y
• JWTLoginHandler line 48
  • force unwrapping jwtParams["domain"]

While these were likely due to configuration issues on my side, these crashes are uncatchable. If something happens in my app in the future, that a variable is not set or overwritten in a weird edge case. I won't be able to provide an error to the user, it will cause the entire app to crash giving a very poor experience. Force unwrapping should be avoided at all costs unless absolutely necessary. Please throw catchable errors instead.

The constructor for SubVerifierDetails has too many optional inputs, making these issues more likely to occur. It would be better to implement convenience inits dedicated to each type of verifier and have required params for it. Asking for dictionaries with unknown key/values, and returning them from all the promise functions, makes using the library difficult unless you know it inside out. Implementing the library and debugging it requires many runs printing out all the objects and turning the logging up to full, just to figure out whats needed to go in, or expected to come out.

Threading

When using FetchNodeDetails.getPublicAddress the below appears in the console and all animation pauses for a second or two. There is a threading issue inside the promiseKit code that will require the calling code to try force this onto a background thread (not sure if possible).

Personally i'm not a fan of PromiseKit as i've had to deal with many such issues in the past. A lot of libraries now make the promisekit definitions optional, by having the main code simply use GCD + completion blocks and having a separate pod Torus+PromiseKit that has extensions. PromiseKit and Promise Foundation is also a large dependency, it would be great to not have to include it in our app just for one dependency.

PromiseKit: warning: `wait()` called on main thread!
PromiseKit: warning: `wait()` called on main thread!
PromiseKit: warning: `wait()` called on main thread!
PromiseKit: warning: `wait()` called on main thread!
PromiseKit: warning: `wait()` called on main thread!
PromiseKit: warning: `wait()` called on main thread!

Documentation

The code samples in this repo differ from the code samples here: https://docs.tor.us/integration-builder/?b=customauth&chain=Ethereum&lang=iOS

The code sample to handle universal links in a storyboard app also doesn't compile. Its missing a return statement and specifies a class type that doesn't exist. Should be NSUserActivity instead of UIUserActivity.

[simonmcl]

Update:

The main thread issues are coming from

FetchNodeDetails(...).getNodeDetails()

This is blocking the main thread. I've had to create a wrapper function around this to chuck it on a background thread so that it stops freezing my UI

[simonmcl]

I've been using the sample verifier credentials in the documentation, until the company i'm working for has time to look into setting up the mobile stuff. The above mentioned crashes were only happening in the simulator with a configuration issue. I've resolved those now and its working in my simulator. Running it on a real device is now crashing 100% of the time with the same error.

I now can't get anything to work and the library is unusable, as any attempt to use it force crashes the app. Please remove the force unwrapping, its extremely bad practise. Add some error enums and give me some indication of why the pub_key_X is missing

Ticket has been open for a month with no comment. Can I get some kind of an update?

cc: @rathishubham7 @tetratorus @YZhenY

[metallicalfa2]

@simonmcl I have added all the above suggestions to our sprint. I'll look into why the pub_key_X is missing and meanwhile, remove the force unwrapping.