Ac.Composite Error Handling Hides Errors from Parent #865

Closed
ooskapenaar opened this Issue Dec 17, 2012 · 3 comments

Comments

Projects
None yet
3 participants

In v0.5.0 composite.common.js we see the following code, which effectively hides any child errors from the caller.

If the parent is to be allowed to "fail-fast", which I think is a good thing, then mojito must pass errors up to the parent and not swallow them like this.

Swallowing of errors by mojito should only be allowed at the top level (i.e. to clean up after a messy parent).

       error: function(err) {
            Y.log("Error executing child mojit at '" + this.id + "':", 'error',
                    NAME);
            if (err.message) {
                Y.log(err.message, 'error', NAME);
            } else {
                Y.log(err, 'error', NAME);
            }
            if (err.stack) {
                Y.log(err.stack, 'error', NAME);
            }
            // Pass back some empty data so we don't fail the composite
            this.done('');

I am going to patch this code, by passing an error object back as the data, then the parent can sort out the mess, an alternative would be to throw an error and disrupt the composite process completely:

           if( err instanceof Error ) {
                this.done( err );
            } else {
                this.done( new Error( err ) );
            }

Opinions, ideas?

@ghost ghost assigned caridy Jan 29, 2013

Contributor

rwaldura commented Feb 11, 2013

see also issue #25

caridy added a commit that referenced this issue Feb 20, 2013

Merge pull request #978 from caridy/composite-errors
Fixes issue #865. Fixes issue #25: `mojito-composite-addon` error propagation by flagging a child with `propagateFailure` flag.
Collaborator

caridy commented Feb 20, 2013

will be available in mojito@0.5.5 scheduled for next week.

@caridy caridy closed this Feb 20, 2013

@caridy caridy removed this from the v0.5.5 milestone Mar 3, 2014

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