CC-Archived / promise-as3

Promises/A+ compliant implementation in ActionScript 3.0
168 stars 55 forks source link

Correct way to handle errors? #47

Closed mikecann closed 9 years ago

mikecann commented 9 years ago

Hi,

I know this library is old and probably not being maintained any more but im working on a legacy project which could really do with some promises to help out.

I have run into an issue however, suppose I have this scenario:

public function testPromiseError():void
{
    throwsAnError()
        .then(function():void { trace("doing something else"); })

        // This doesnt get called because the promise chain hasnt yet been setup!
        .otherwise(function() : void { trace("whoops error occured!"); });
}

public function throwsAnError() : Promise
{
    // This represents some operation that could potentially throw an errror
    throw new Error("Whoops!");

    return Deferred.resolve(true);
}

Notice that because the error is thrown before the otherwise is added to the promise chain, my handler doesnt get called. As far as I can tell there are two ways to solve this.

1) Defer the initial call so the promise chain can be setup:

public function testPromiseError():void
{
    Deferred.resolve(true)
        .then(throwsAnError)
        .then(function():void { trace("doing something else"); })

        // This now gets called!
        .otherwise(function() : void { trace("whoops error occured!"); });
}

public function throwsAnError() : Promise
{
    // This represents some operation that could potentially throw an errror
    throw new Error("Whoops!");

    return Deferred.resolve(true);
}

or option 2) delay the body of the function:

public function testPromiseError():void
{
    throwsAnError()
        .then(function():void { trace("doing something else"); })
        .otherwise(function() : void { trace("whoops error occured!"); });
}

public function throwsAnError() : Promise
{
    return Promise.delay(true, 10)
        .then(function():Promise{
            // This represents some operation that could potentially throw an errror
            throw new Error("Whoops!");

            return Deferred.resolve(true);
        });         
}

Both seem kind of hacky, which is the recommended solution?

karfau commented 9 years ago

I would say use try catch as normally

public function throwsAnError() : Promise
{
    try{
        // This represents some operation that could potentially throw an errror
        throw new Error("Whoops!");

        return Deferred.resolve(true);
    }catch(expected:Error){
        return Deferred.reject(expected);
    }
    //maybe the successful return needs to go here for some AS compilers to recognize the return.
}

This code was not executed, so take it as pseudo code ;)

johnyanarella commented 9 years ago

+1 to what Christian suggests

mikecann commented 9 years ago

Okay cool, that makes sense I guess. Its not as neat as I was hoping for but logically it makes sense.

Thanks!

karfau commented 9 years ago

Depending on what that error throwing code is (did you wirte it, part of your application or not, etc.) you might be able to avoid the Error being thrown be checking something first and rejecting in that case. e.g. to avoid (one) Nullpointer. But I think the cases where this makes more sense on all levels are really edge cases. If you want the method to always return a Promise (which makes it a clean and easy to use API) you might want to put a try catch around the whole body anyway. But of course you can also let the user of your method do the try catch, if this is what you wantin some cases.

I was just reading a lot of stuff about this topic in the JS world after your input, thank you for this. I especially liked this one: http://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html