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

How is error handling supposed to work? #89

Closed
kyeotic opened this issue Oct 10, 2017 · 9 comments
Closed

How is error handling supposed to work? #89

kyeotic opened this issue Oct 10, 2017 · 9 comments

Comments

@kyeotic
Copy link

kyeotic commented Oct 10, 2017

When running yeoman environment errors in generators get swallowed. I've trying using env.on('error') to register a handler, but it does not get invoked when a generator throws an error. The error param in the run() callback isn't set either. What's going on? How do we catch errors?

@SBoudrias
Copy link
Member

SBoudrias commented Oct 11, 2017

Hey @tyrsius, it is very hard to see what is the issue you're running into. Usually errors aren't swallowed and exposed properly to the user; so you're definitely not hitting the desired/default behavior .

As such, I can't help you find/fix the bug unless you provide a more complete example. Can you provide the exact code and steps to reproduce your issue?

@kyeotic
Copy link
Author

kyeotic commented Oct 11, 2017

Sure. Ive got a generator that composes other generators.

module.exports = class extends Generator {
    initializing() {
        // Make this available for all plugins to extend
        this.options.composer = new composer_1.default({
            projectName: this.options.projectName,
            namespace: this.options.namespace
        });
        this.destinationRoot(path.resolve(this.destinationRoot(), this.options.outputDir, outputSuffix));
        // Add all the plugins
        Object.keys(this.options.plugins).forEach(key => {
            console.log('handling', key, getPathForPlugin(key));
            let plugin = this.options.plugins[key];
            this.composeWith(getPathForPlugin(key), Object.assign({}, this.options, { inputs: plugin }));
        });
    }
};

And a runner that kicks it off

function runComposer(outputDir, options) {
    const yoEnv = yeoman.createEnv();
    yoEnv.registerStub(ComposerGenerator, 'ComposerGenerator');
    let zipFile = path.resolve(outputDir, zipName);
    let zipFolderPath = path.resolve(outputDir, outputSuffix);
    return new Promise((resolve, reject) => {
        yoEnv.run('ComposerGenerator', options, (err) => {
            if (err)
                return reject(err);
            zip_1.default(zipFolderPath, zipFile, (err) => {
                if (err)
                    return reject(err);
                console.log('composition complete');
                resolve(zipFile);
            });
        });
    }).catch(err => {
        console.error(err, err.stack);
        throw err;
    });
}

If any of the composed generators throws errors in their normal functions, like writing for example, those errors do not get returning to the err on the run() call. If I setup an error listenting with yoEnv.on('error') that is not invoked either.

@kyeotic
Copy link
Author

kyeotic commented Oct 12, 2017

@SBoudrias ☝️

@SBoudrias
Copy link
Member

@tyrsius so if I understand this properly; only errors thrown inside composed generators are not being catch?

@kyeotic
Copy link
Author

kyeotic commented Oct 15, 2017

Yes.

@SBoudrias
Copy link
Member

Okay, this looks like a valid bug. I don't think I'll have much time to dig into it in the next week. But let me know if I can help you prepare a PR to fix that!

@regevbr
Copy link
Contributor

regevbr commented Jul 11, 2018

This should be an easy fix - just register the error event of created generators and emit error on the environment itself.
Temp fix:

    const generator = env.create(generatorName, { args: [], options: {} });
    if (generator instanceof Error) {
      return onError(generator);
    }
    generator.on('error', onError);
    generator.run((err) => {
      // error was handled already in the event handler above
      if (!err){
        console.log('Finished generating');
        resolve();
      }
    });

@SBoudrias
Copy link
Member

@regevbr want to send a PR for this one? Looks like you got it figured out

@regevbr
Copy link
Contributor

regevbr commented Jul 15, 2018

@SBoudrias sure ill give it a go

SBoudrias pushed a commit that referenced this issue Jul 17, 2018
* Fix generator error handling (Fix #89)

* Fix generator error handling (Fix #89)

* Fix generator error handling (Fix #89)
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