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

.catch doesn't fire if .then fails #162

Open
appsforartists opened this Issue Oct 21, 2014 · 3 comments

Comments

Projects
None yet
3 participants
@appsforartists

appsforartists commented Oct 21, 2014

Reqwest's then implementation is faulty. It should be wrapped in a try/catch and pass the error onto the promise's catch method, per the spec.

For comparison, here's the correct behavior (verified using Node's in-built promises):

> var promise = new Promise(function (resolve, reject) {global.resolve = resolve})
undefined
> resolve
[Function]
> promise.then(function () { throw new Error("hi") }).catch(function(error) { console.log("caught " + error)})
{}
> resolve()
undefined
> caught Error: hi
@wbobeirne

This comment has been minimized.

Show comment
Hide comment
@wbobeirne

wbobeirne Jul 25, 2016

Confirming this isn't working, and is particularly frustrating for if you want to save code and just trigger the catch case in your then function. I.e.

reqwest({
    url: ajaxPath,
    data: ajaxParams,
})
.then((res) => {
    if (res && res.data) {
        hellYeahData(res.data);
    }
    else {
        throw new Error(res);
    }
})
.catch((err) => {
    if (err && err.message) {
        ohNoShowError(err.message);
    }
    else {
        ohNoShowError("Something went wrong, try again.");
    }
});

wbobeirne commented Jul 25, 2016

Confirming this isn't working, and is particularly frustrating for if you want to save code and just trigger the catch case in your then function. I.e.

reqwest({
    url: ajaxPath,
    data: ajaxParams,
})
.then((res) => {
    if (res && res.data) {
        hellYeahData(res.data);
    }
    else {
        throw new Error(res);
    }
})
.catch((err) => {
    if (err && err.message) {
        ohNoShowError(err.message);
    }
    else {
        ohNoShowError("Something went wrong, try again.");
    }
});
@appsforartists

This comment has been minimized.

Show comment
Hide comment
@appsforartists

appsforartists Jul 25, 2016

I haven't touched reqwest in a really long time because fetch is a thing now.

appsforartists commented Jul 25, 2016

I haven't touched reqwest in a really long time because fetch is a thing now.

@danhardman

This comment has been minimized.

Show comment
Hide comment
@danhardman

danhardman Jul 26, 2016

@appsforartists Possibly the best comment on this repository in months. Thanks for the heads up (y)

danhardman commented Jul 26, 2016

@appsforartists Possibly the best comment on this repository in months. Thanks for the heads up (y)

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