stellar-deprecated / stellar-client

INACTIVE. Browser based client for stellard. This repository is inactive. It points to the stellard network, which is being replaced by stellar-core. Please refer to the replacement repository, interstellar-client, which points to the stellar-core network.
Other
306 stars 74 forks source link

Minor race condition in trading balance widget when user changes currency pair while still loading #893

Open irisli opened 9 years ago

irisli commented 9 years ago

In the trading balance widget, when a user selects a currency pair, the client will go to fetch the balances and then show the balances of the pair:

race-condition

If during this loading state (due to a user's high latency for example in a rural place), the user changes the currency pair, the client will still load and show the balances for the old one.

This is because trading-balance-controller.js' refreshBalances is a singletonPromise.

If it were just a normal function, the race condition could still occur if the user changes a pair while still loading (again, on a high latency device) and the second request finishes first. Once the first request for balances comes back, the promise would be fulfilled and the balance would show the old pair.

Bug frequency

This bug could actually show quite frequently if someone selects a currency and then tabs to switch to a different issuer. A user might do this while having a latency of 300ms (quite common with 3G). The race condition shouldn't be blamed on user error for them being too fast.

irisli commented 9 years ago

From a discussion:

Use promise.allSettled() and implement a way of cancelling a promise. We let the promise run and (before updating comment) at the end we reject and exit if a new action has occured.

Also make singletonPromise for non-button initiated actions.

deckar01 commented 9 years ago

Time-box exhausted for now. Supporting the isLoading() on the wrapper and also being able interrupting the promise chain of its returned value proved to be more complicated than expected.

The constraints end up creating an interface like

$scope.fun = repeatedPromise(function() {
  return promise;
}).then(next);

or

$scope.fun = repeatedPromise(function() {
  return cancelablePromise(promise).then(next);
});

because we need to get in between the core resource request and the promise chain.

Along the way I found: The interface Angular provides for canceling $http requests is timeout, which accepts a promise that when resolved will abort the request.

deckar01 commented 9 years ago

CancelablePromise was implemented during the send refactor and should be used in place of the singletonPromise when preempting a resource request that does not originate from a user interaction (timers, websocket messages).