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

Errors while loading plugins are hidden #254

Closed
bkonkle opened this issue Jun 7, 2014 · 6 comments
Closed

Errors while loading plugins are hidden #254

bkonkle opened this issue Jun 7, 2014 · 6 comments

Comments

@bkonkle
Copy link

bkonkle commented Jun 7, 2014

As identified in previous issues, async 0.2 prevents callbacks from being invoked more than once by throwing an error. In 'core/environment', this causes an issue when there is an error while loading a plugin.

In the line I linked the 'done' function has already been called (which calls the callback), but the 'catch' block tries to call it again with the error. This results in a traceback like this that hides the original error:

/Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:22
            if (called) throw new Error("Callback was already called.");
                              ^
Error: Callback was already called.
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:22:31
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:229:17
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:516:34
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:232:13
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:136:21
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:229:17
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:556:34
    at /Users/brandon/code/lldotcom-2014/node_modules/wintersmith/node_modules/async/lib/async.js:136:21
    at done (/Users/brandon/code/lldotcom-2014/node_modules/wintersmith/lib/core/environment.js:260:16)
    at Environment.loadPluginModule (/Users/brandon/code/lldotcom-2014/node_modules/wintersmith/lib/core/environment.js:277:16)

Unfortunately, I'm not sure how to solve it. If someone can give me a push in the right direction, I'd be glad to work up a pull request. :-)

@bkonkle
Copy link
Author

bkonkle commented Jun 11, 2014

In the meantime, while developing plugins I'm removing the try/catch block and just letting errors bubble up so that I can see what went wrong.

@jnordberg
Copy link
Owner

Is your plugin somehow managing to throw and call the callback?

@bkonkle
Copy link
Author

bkonkle commented Jul 7, 2014

This isn't just specific to my plugins - any time any plugin throws any kind of error, it's swallowed by this async error. For example, I've had errors related to a misconfigured pagination plugin (taken from the examples in this repo), but I had no idea that's what was actually going wrong until I removed the try/catch block.

@jnordberg
Copy link
Owner

Can you post one of your plugins or some code to reproduce?

@bkonkle
Copy link
Author

bkonkle commented Sep 10, 2014

The project I was working on isn't public at the moment, but here's a quick example:

/*
 * A subclass of Wintersmith's MarkdownPage plugin, customized to provide the
 * behavior we want for the blog.
 */
'use strict';

var _ = require('underscore');
var CategoryGenerator = require('./CategoryGenerator');
var extend = require('../utils/extend');
var getContent = require('../utils/getContent');
var PaginatorGenerator = require('./PaginatorGenerator');
var path = require('path');

var plugin = function(env, callback) {
  var BlogPost = extend.call(env.plugins.MarkdownPage, {

    getTemplate: function() {
      // Allow the template to be overridden, but default to one specific to
      // blog posts
      return this.metadata.template || 'blog/post.html';
    },

    getFilename: function() {
      if (this.metadata.slug) {
        // Allow for a slug-based override
        return 'blog/' + this.metadata.slug + '/index.html';
      } else {
        // Otherwise, use just the filename (ignoring the year) as the slug
        var basename = path.basename(this.filepath.relative);
        return '/blog/' + env.utils.stripExtension(basename) + '/index.html';
      }
    },

    getAuthor: function(contents) {
      // A convenience method to get the author object related to the blog
      // post.
      return contents['team'][this.metadata.author + '.md'];
    },

    getCategories: function() {
      // Split the categories by commas, trim the whitespace, & force lowercase
      return this.metadata.categories.split(',').map(function(cat) {
        return cat.trim().toLowerCase();
      });
    }

  });

  // Register the new plugin, limiting it to entries in the 'blog' folder
  env.registerContentPlugin('pages', 'blog/**/*.*(markdown|mkd|md)', BlogPost);

  function getBlogPosts(contents) {
    return getContent(contents['blog'], BlogPost);
  }

  function getBlogPostsForCategory(contents, category) {
    var posts = getContent(contents['blog'], BlogPost);

    // Filter the list to contain only posts that have the requested category
    return _.filter(posts, function(post) {
      // Split the categories by commas, trim the whitespace, & force lowercase
      var categories = post.getCategories();

      // Check to see if the list of categories contains the one requested
      return _.contains(categories, category.toLowerCase());
    });
  }

  // Export the blog post helper to the environment
  env.helpers.getBlogPosts = getBlogPosts;
  env.helpers.getBlogPostsForCategory = getBlogPostsForCategory;

  // Create a generator for the blog index
  env.registerGenerator('blogIndex', PaginatorGenerator.create(env, {
    perPage: 20,
    baseDir: 'blog',
    pageDir: 'page',
    template: 'blog/list.html'
  }, getBlogPosts));

  // Create a generator for the category indexes
  env.registerGenerator('categoryIndexes', CategoryGenerator.create(
    env, getBlogPosts, getBlogPostsForCategory
  ));

  callback();
};

module.exports = plugin;

For the record, here's the 'extend' function:

/*
 * Backbone's extend functionality, which is almost identical to CoffeeScript's
 * method. This will make it easy to extend classes originally created in
 * CoffeeScript.
 *
 * From: https://github.com/jashkenas/backbone/blob/c33dcdeffd85a5b749100249b8b9de7be44a0594/backbone.js#L1648
 */
'use strict';

var _ = require('underscore');

var extend = function(protoProps, staticProps) {
  var parent = this;
  var child;

  // The constructor function for the new subclass is either defined by you
  // (the "constructor" property in your `extend` definition), or defaulted
  // by us to simply call the parent's constructor.
  if (protoProps && _.has(protoProps, 'constructor')) {
    child = protoProps.constructor;
  } else {
    child = function(){ return parent.apply(this, arguments); };
  }

  // Add static properties to the constructor function, if supplied.
  _.extend(child, parent, staticProps);

  // Set the prototype chain to inherit from `parent`, without calling
  // `parent`'s constructor function.
  var Surrogate = function(){ this.constructor = child; };
  Surrogate.prototype = parent.prototype;
  child.prototype = new Surrogate();

  // Add prototype properties (instance properties) to the subclass,
  // if supplied.
  if (protoProps) _.extend(child.prototype, protoProps);

  // Set a convenience property in case the parent's prototype is needed
  // later.
  child.__super__ = parent.prototype;

  return child;
};

module.exports = extend;

@jnordberg
Copy link
Owner

If your plugin loading method throws after calling the callback it could cause it to be called twice, I would argue that is an implementation error but it's probably a good idea to have a check in the done function that only logs any subsequent calls to it with an error instead of calling the callback multiple times.

Can't at a glance see how it could happen with the code you posted however...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants