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.

Sign up for GitHub

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

Jump to bottom

is it fine if onError, onSuccess is invoked after the computation is canceled? #148

Closed
safareli opened this issue May 8, 2018 · 4 comments · May be fixed by #149
Closed

is it fine if onError, onSuccess is invoked after the computation is canceled? #148

safareli opened this issue May 8, 2018 · 4 comments · May be fixed by #149

Comments

@safareli
Copy link
Contributor

safareli commented May 8, 2018

The lib i'm wrapping has no cancellation, this is what i have now:

exports._sendAsync = function (provider) {
    return function (request) {
        return function(onError, onSuccess) {
            var canceled = false;
            provider.sendAsync(request, function(err, succ) {
                if (canceled) {
                    return;
                } else if (err) {
                    onError(err);
                } else {
                    onSuccess(succ);
                }
            });
            return function (cancelError, onCancelerError, onCancelerSuccess) {
                canceled = true;
                onCancelerSuccess();
            };
        };
    };
};

my question is, is the canceled staff required, or this version will be fine too:

exports._sendAsync = function (provider) {
    return function (request) {
        return function(onError, onSuccess) {
            provider.sendAsync(request, function(err, succ) {
                if (err) {
                    onError(err);
                } else {
                    onSuccess(succ);
                }
            });
            return function (cancelError, onCancelerError, onCancelerSuccess) {
                onCancelerSuccess();
            };
        };
    };
};
@safareli safareli mentioned this issue May 8, 2018
Merged
@natefaubion
Copy link
Collaborator

The Aff runtime guards against reentry so the latter is fine. The upstream library should probably be fixed though. Since it's using XHR, it can definitely be cancelled, it just doesn't expose the request to the user.

@safareli
Copy link
Contributor Author

safareli commented May 8, 2018

Thanks, opened pr to upstream lib too.

Also if onError or onSuccess is invoked then canceler function will not be invoked too right?

@natefaubion
Copy link
Collaborator

It is not possible to invoke a canceler after the callback is resolved.

safareli added a commit to safareli/purescript-aff that referenced this issue May 9, 2018
@safareli
Add note about some runtime guaranties
c201010 
closes purescript-contrib#148
@safareli safareli mentioned this issue May 9, 2018
Open
@safareli safareli mentioned this issue Sep 25, 2019
Open
@mushishi78
Copy link

Just to check, if there's no cancel, is it better to say the cancel was a success or failure? The example calls onCancelerSuccess, but could call onCancelerError?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add note about some runtime guaranties safareli/purescript-aff
3 participants
@natefaubion @mushishi78 @safareli