Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestHelper.Abort() throws exception #103

Open
cetonek opened this issue Nov 21, 2019 · 9 comments
Open

RequestHelper.Abort() throws exception #103

cetonek opened this issue Nov 21, 2019 · 9 comments

Comments

@cetonek
Copy link

cetonek commented Nov 21, 2019

Hi, I want to discard all requests that are made as soon as I leave given scene, so my code is:


    public class LadderRepository : MonoBehaviour {

        private string uri = "my uri";

        private readonly IList<RequestHelper> requests = new List<RequestHelper>();

        public IPromise<LadderResponse> FetchLadder() {
            var requestHelper = ComposeRequest(ladder);
            return RestClient.Get<LadderResponse>(requestHelper);
        }

        private RequestHelper ComposeRequest(string url) {
            var requestHelper = new RequestHelper {
                Uri = url,
                Timeout = 10
            };
            requests.Add(requestHelper);
            return requestHelper;
        }

        private void OnDestroy() {
            foreach (var requestHelper in requests) {
                requestHelper.Abort();
            }

            requests.Clear();
        }

    }

I get exception in editor when calling requestHelper.Abort() :

ArgumentNullException: Value cannot be null.
Parameter name: _unity_self
Proyecto26.RequestHelper.Abort () (at Assets/RestClient/Packages/Proyecto26.RestClient/Helpers/RequestHelperExtension.cs:129)
Script.LadderRepository.OnDestroy () (at Assets/Script/Ladder/LadderRepository.cs:30)

I have unity 2019.2.13f1 and RestClient version 2.6.0. My idea is I dont want to receive any callbacks from any requests after user leaves given scene, cant tell what this error means though.

@jdnichollsc
Copy link
Member

Very odd because we're validating here https://github.com/proyecto26/RestClient/blob/master/src/Proyecto26.RestClient/Helpers/RequestHelperExtension.cs#L124

@cetonek
Copy link
Author

cetonek commented Nov 22, 2019

Yeah that looks reasonable, in fact I added check even for !isDone, but it shouldnt change anything. Everytime I tried to access anything in the UnityWebRequest instance I got that exception

@jdnichollsc
Copy link
Member

I'm not sure what validation to add here to fix the exception thrown by the UnityWebRequest instance 🤔

@wloczykij89
Copy link

I've the same problem when I check the download progress of a file that has already finished downloading. It looks like you're referring to UnityWebRequest object after it has been disposed by using block.

https://forum.unity.com/threads/argumentnullexception-appear-randomly-in-unitywebrequest.541629/#post-3856987

@jdnichollsc
Copy link
Member

jdnichollsc commented Mar 9, 2020

Please attach any reproducible demo to debug 🙂

@wloczykij89
Copy link

wloczykij89 commented Mar 10, 2020

I really appreciate your work. It's a really good piece of code. Thanks man :) The following example downloads an image and after that checks progress and call Abort() with some delay. This actions cause errors.
After download success UnityWebRequest was automatically disposed and can no longer be used.

using System;
using System.Collections;
using System.IO;
using Proyecto26;
using UnityEngine;
using UnityEngine.Networking;

public class DownloadTest : MonoBehaviour
{
    public static readonly string TestFilePath = "https://upload.wikimedia.org/wikipedia/commons/3/37/African_Bush_Elephant.jpg";
    private RequestHelper request;

    private void OnEnable()
    {
        Download(TestFilePath);
    }

    public void Download(string path)
    {
        string fileName = Path.GetFileName(path);
        string targetPath = Path.Combine(Application.persistentDataPath, fileName);

        var downloadHandler = new DownloadHandlerFile(targetPath);

        request = new RequestHelper
        {
            Uri = path,
            DownloadHandler = downloadHandler
        };

        RestClient.Get(request)
            .Then((ResponseHelper obj) =>
            {
                Debug.Log("Download success");

                StartCoroutine(DelayAbort(2));
            })
            .Catch((Exception error) =>
            {
                Debug.LogError("Download failed: " + error.Message);
            });
            
        StartCoroutine(CheckProgress());
    }

    public IEnumerator DelayAbort(float delay)
    {
        yield return new WaitForSeconds(delay);

        request.Abort();
    }

    private IEnumerator CheckProgress()
    {
        for(; ; )
        {
            var progress = request.DownloadProgress;

            yield return null;
        }
    }
}

@jdnichollsc
Copy link
Member

I think you can't call Abort method from Then because the request has finished, also you can verify if the request is aborted using request.IsAborted

@wloczykij89
Copy link

The problem is not just about call Abort().
The above code is only a reproduction of @Ejstn's case. In his code in the OnDestroy method, all requests are aborted, both those completed and those that are still ongoing.
For requests that were completed, the UnityWebRequest object was automatically Dispose() - using block calls Dispose() uppon leaving the block.
In that case in RequestHelper checking if(Request != null) returns true but performing any operation on this object is forbidden and will throw ArgumentNullException (Request.isDone, Request.Abort() ...).

@jdnichollsc
Copy link
Member

jdnichollsc commented Mar 10, 2020

Thanks mate, let me check, also any pull request is welcome! <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants