siweAuth returns LogInResult where updatesCurrentAsOf is of type {+[keyserverID: string]: number}. response.serverTime is a number. Flow didn't catch this because to it response is any. We use ashoatKeyserverID because this action was not yet refactored to work with multiple keyservers.
Details
Checked that the types are now correct
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Thanks! Would've been good to catch this in D10058
Do you know if this would have caused any issues on SIWE clients? Eg. would there be a crash loop because updatesCurrentAsOf gets set to 0 or undefined?
Also, curious why password login (logIn) does a fanout to multiple keyservers, but SIWE login (siweAuth) only goes to one keyserver at a time. I guess siweAuth just hasn't been refactored yet to multi-keyserver... is there a task tracking this?
(I wonder if perhaps the approach in logIn will be replaced by ENG-5997, where we'll have a separate instance of a handler component for each keyserver, that will perhaps handle login individually for each keyserver, and will do it via a new keyserver endpoint that @varun introduces in ENG-3262. As such, not sure if it makes sense to do the above refactoring on siweAuth. cc @tomek)
Do you know if this would have caused any issues on SIWE clients? Eg. would there be a crash loop because updatesCurrentAsOf gets set to 0 or undefined?
This would result in full state sync, which I think can cause a crash loop, yes.
Also, curious why password login (logIn) does a fanout to multiple keyservers, but SIWE login (siweAuth) only goes to one keyserver at a time. I guess siweAuth just hasn't been refactored yet to multi-keyserver... is there a task tracking this?
As discussed here, SIWE actions are supposed to be deprecated by @varun's work. I think this is tracked in ENG-5926. Login was refactored by me to make it easier for @varun to add the new login action, that would use the new way of calling keyservers with useKeyserverCall, that I had implemented.
Thanks for explaining, @inka! I'll try to make a new native (and web) release after this is landed