1
0
mirror of https://github.com/ppy/osu.git synced 2025-01-06 04:13:11 +08:00

Fix aborting an APIRequest potentially resulting in incorrect success

This commit is contained in:
Dean Herbert 2021-07-28 23:58:57 +09:00
parent 2a94fc214f
commit 3a5324c947

View File

@ -86,8 +86,6 @@ namespace osu.Game.Online.API
/// </summary> /// </summary>
private APIRequestCompletionState completionState; private APIRequestCompletionState completionState;
private Action pendingFailure;
public void Perform(IAPIProvider api) public void Perform(IAPIProvider api)
{ {
if (!(api is APIAccess apiAccess)) if (!(api is APIAccess apiAccess))
@ -99,29 +97,23 @@ namespace osu.Game.Online.API
API = apiAccess; API = apiAccess;
User = apiAccess.LocalUser.Value; User = apiAccess.LocalUser.Value;
if (checkAndScheduleFailure()) if (isFailing) return;
return;
WebRequest = CreateWebRequest(); WebRequest = CreateWebRequest();
WebRequest.Failed += Fail; WebRequest.Failed += Fail;
WebRequest.AllowRetryOnTimeout = false; WebRequest.AllowRetryOnTimeout = false;
WebRequest.AddHeader("Authorization", $"Bearer {API.AccessToken}"); WebRequest.AddHeader("Authorization", $"Bearer {API.AccessToken}");
if (checkAndScheduleFailure()) if (isFailing) return;
return;
if (!WebRequest.Aborted) // could have been aborted by a Cancel() call Logger.Log($@"Performing request {this}", LoggingTarget.Network);
{ WebRequest.Perform();
Logger.Log($@"Performing request {this}", LoggingTarget.Network);
WebRequest.Perform();
}
if (checkAndScheduleFailure()) if (isFailing) return;
return;
PostProcess(); PostProcess();
API.Schedule(TriggerSuccess); TriggerSuccess();
} }
/// <summary> /// <summary>
@ -141,7 +133,10 @@ namespace osu.Game.Online.API
completionState = APIRequestCompletionState.Completed; completionState = APIRequestCompletionState.Completed;
} }
Success?.Invoke(); if (API == null)
Success?.Invoke();
else
API.Schedule(() => Success?.Invoke());
} }
internal void TriggerFailure(Exception e) internal void TriggerFailure(Exception e)
@ -154,7 +149,10 @@ namespace osu.Game.Online.API
completionState = APIRequestCompletionState.Failed; completionState = APIRequestCompletionState.Failed;
} }
Failure?.Invoke(e); if (API == null)
Failure?.Invoke(e);
else
API.Schedule(() => Failure?.Invoke(e));
} }
public void Cancel() => Fail(new OperationCanceledException(@"Request cancelled")); public void Cancel() => Fail(new OperationCanceledException(@"Request cancelled"));
@ -163,59 +161,47 @@ namespace osu.Game.Online.API
{ {
lock (completionStateLock) lock (completionStateLock)
{ {
// while it doesn't matter if code following this check is run more than once,
// this avoids unnecessarily performing work where we are already sure the user has been informed.
if (completionState != APIRequestCompletionState.Waiting) if (completionState != APIRequestCompletionState.Waiting)
return; return;
}
WebRequest?.Abort(); WebRequest?.Abort();
// in the case of a cancellation we don't care about whether there's an error in the response. // in the case of a cancellation we don't care about whether there's an error in the response.
if (!(e is OperationCanceledException)) if (!(e is OperationCanceledException))
{
string responseString = WebRequest?.GetResponseString();
// naive check whether there's an error in the response to avoid unnecessary JSON deserialisation.
if (!string.IsNullOrEmpty(responseString) && responseString.Contains(@"""error"""))
{ {
try string responseString = WebRequest?.GetResponseString();
{
// attempt to decode a displayable error string. // naive check whether there's an error in the response to avoid unnecessary JSON deserialisation.
var error = JsonConvert.DeserializeObject<DisplayableError>(responseString); if (!string.IsNullOrEmpty(responseString) && responseString.Contains(@"""error"""))
if (error != null)
e = new APIException(error.ErrorMessage, e);
}
catch
{ {
try
{
// attempt to decode a displayable error string.
var error = JsonConvert.DeserializeObject<DisplayableError>(responseString);
if (error != null)
e = new APIException(error.ErrorMessage, e);
}
catch
{
}
} }
} }
}
Logger.Log($@"Failing request {this} ({e})", LoggingTarget.Network); Logger.Log($@"Failing request {this} ({e})", LoggingTarget.Network);
pendingFailure = () => TriggerFailure(e); TriggerFailure(e);
checkAndScheduleFailure(); }
} }
/// <summary> /// <summary>
/// Checked for cancellation or error. Also queues up the Failed event if we can. /// Whether this request is in a failing or failed state.
/// </summary> /// </summary>
/// <returns>Whether we are in a failed or cancelled state.</returns> private bool isFailing
private bool checkAndScheduleFailure()
{ {
lock (completionStateLock) get
{ {
if (pendingFailure == null) lock (completionStateLock)
return completionState == APIRequestCompletionState.Failed; return completionState == APIRequestCompletionState.Failed;
} }
if (API == null)
pendingFailure();
else
API.Schedule(pendingFailure);
pendingFailure = null;
return true;
} }
private class DisplayableError private class DisplayableError