[WIP] check if Onyx is ready when app is authenticating

[marcochavezf]

Problem

There's an issue on Android where the user is logged out randomly. We're not 100% sure about the root cause, but one possibility is the existence of a race condition between the async storage and the networks requests; the Onyx.connect callback to retrieve the stored authToken could be called after the Network.registerParameterEnhancer callbacks (which adds the authToken in the requests) and if the authToken is not ready when a request is made then the user is logged out.

I was able to reproduce this scenario by adding a setTimeout in the Onyx.connect callback (more details in this comment):

Screen.Recording.2021-10-12.at.18.32.38.mov

Proposed workaround and solution

I think the long-term solution should be a restructuring of the handling between the async storage and the network layer in order to wait first for the async storage (Onyx) to be resolved to then set up the network layer.

But since we're only 100% sure if this race condition exists and is the cause of the random logout I propose to add a flag to check if authToken is ready from Onyx and log to the server a message if the race condition happens. This flag also skips adding the default parameters when Onyx is not ready; in that way, we avoid making a request without the authToken (which hasn't been retrieved from Onyx) and thus avoid logging out the user.

If the race condition exists, the message Onyx is not ready in Network.registerParameterEnhancer is logged to the server, so with that can be sure that:

• A. Onyx not being ready could be the real issue and will require restructuring the app bootstrapping to wait first for Onyx to be ready before making any network request.

  or

• B. If the random logout is still happening and the message is not found in the logs we ensure that a delay in Onyx.connect is not the culprit of the random logout.

---

Fixed Issues

$ #5619

Tests

QA Steps

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

[kidroca]

IMO this doesn't solve the root problem we're just skipping adding the default parameters when we can't

If we're making a request that needs an auth token it will just fail, which would trigger re-authentication and so on
We could have a perfectly valid token and this will forfeit it and delay the original request even more

It adds a complication that requires significant explanation, and this just solves the authToken case, while we have other keys like credentials that might be suffering from the same problem

Obtaining (and waiting for) the storage values right before the request is made is the right way to go IMO
At that point you're already in an async operation - the request call. You can start another async operation like reading from storage and then chain the request to it. Reading from storage would either happen instantly (if the key is cached) or will take some time.
This way the code is self-explanatory - we don't have to explain that Onyx or the token is not ready

[kidroca]

Not returning here means the network call is probably losing it's parameters

App/src/libs/Network.js

Lines 199 to 213 in 886aa24

	const finalParameters = _.isFunction(enhanceParameters)
	? enhanceParameters(queuedRequest.command, requestData)
	: requestData;
	// Check to see if the queue has paused again. It's possible that a call to enhanceParameters()
	// has paused the queue and if this is the case we must return. We don't retry these requests
	// since if a request is made without an authToken we sign out the user.
	if (!canMakeRequest(queuedRequest)) {
	return;
	}
	HttpUtils.xhr(queuedRequest.command, finalParameters, queuedRequest.type, queuedRequest.shouldUseSecure)
	.then(response => onResponse(queuedRequest, response))
	.catch(error => onError(queuedRequest, error));
	});

If the idea is to acknowledge and still let the request to go through without the default values, we should at least return the parameters here

[marcochavezf]

IMO this doesn't solve the root problem we're just skipping adding the default parameters when we can't

If we're making a request that needs an auth token it will just fail, which would trigger re-authentication and so on We could have a perfectly valid token and this will forfeit it and delay the original request even more

Thanks for the review @kidroca, you're right. I'm going to close this PR and reopen a new one with another solution.

while we have other keys like credentials that might be suffering from the same problem

About the credentials, I'm not really sure if that cause the logout issue, because I've been testing without the credentials data and the app keeps working fine, for example here I even commented out the code to retrieve the credentials and the user is still logged in:

[kidroca]

About the credentials, I'm not really sure if that cause the logout issue, because I've been testing without the credentials data and the app keeps working fine

They will only matter when the user needs to reauthenticate. On app launch If the token expired but credentials haven’t been read yet
I think there might be a small chance of that happening