app.render silent fail

[rhettl]

Hello, This issue is very similar to #354, but I didn't want to force that re-opened.

I am running into an issue that app.render silent fails.

The website I am writing is sending reminder emails 14 days and 2 days before scheduled events. So the emails are being triggered by setTimeout and there is not Response object from which to copy res.locals.

I have tried adding _locals, contentLocals, context.locals, and _locals.context.localities properties to my data object to send my default 'en-US' locality to dust/makara, but with no success. I have even traced to see dust-makara-helpers check for the right variables in localeFromContext to return my 'en-US' locality to the loader function, but somewhere there after it still fails and I cannot see where.

Here is my email module

var extend = require('extend');
var nodemailer = require('nodemailer');
var sendmailTransport = require('nodemailer-sendmail-transport');

var baseDefaults = {};
var baseTemplateDefaults = {
  //_locals: {
  //  contentLocale: {
  //    locale: 'en-US'
  //  }
  //}
};

module.exports = {
  __render: null,
  __app: null,
  __defaults: {},
  __templateDefaults: {},
  /**
   * Setup MailTransport with nodemailer
   * @param app  {object} kraken/express application
   * @param conf {object} mailtransport configuration settings from config.js
   */
  config: function (app, conf) {
    if (!app || !app.render) {
      throw new Error('There is no render function. Please fix it. Fix it, fix it now!');
    }
    if (!conf || !conf.name || !module.exports['__'+conf.name]) {
      throw new Error('No transport has been named or named transport doesn\'t exist.');
    }

    module.exports.__app = app;
    module.exports.__render = app.render.bind(app);
    module.exports.__transport = module.exports['__' + conf.name](conf.options);
    module.exports.__defaults = extend({}, baseDefaults, conf.defaults);
    module.exports.__templateDefaults = extend({}, baseTemplateDefaults, conf.templateDefaults);
    console.log('Mail Transport Ready');
  },

  __sendmail: function(options){
    return nodemailer.createTransport(sendmailTransport(options));
  },


  render: function(template, data, callback) {
    if (typeof data === 'function') {
      if (!callback) {
        callback = data;
        data = {};
      } else if (typeof callback === 'function') {
        //noinspection JSUnresolvedFunction
        data = data() || {};
      }
    }

    var tempData = extend({}, module.exports.__templateDefaults, data);

    try{
      module.exports.__render('emails/' + template, tempData, callback);
    } catch (err) {
      callback(err);
    }
   },

  send: function(template, templateData, mailOptions, callback){
    module.exports.render(template, templateData, function(err, html){
      console.log('here I am');
      if (err) {
        return callback(err);
      }

      var optionsCombined = extend({html: html}, module.exports.__defaults, mailOptions);
      //console.log(html, optionsCombined);


      console.log(optionsCombined);
      module.exports.__transport.sendMail(optionsCombined, callback || function(){});
    });
  }

};

and a section of my config.json (with some minor modification of the personal information)

{
  "mailtransport": {
    "name": "sendmail",
    "options": {
      "path": "/usr/sbin/sendmail",
      "args": ["-i", "-t"]
    },
    "defaults": {
      "from": "Rhett <rhett@******.edu>",
      "subject": "[*******] Interviews App",
      "generateTextFromHTML": true
    },
    "templateDefaults": {
      "domain": "******.edu:8000"
    }
  }
}

Thank you for any help you might be able to provide. I have been racking my brain on this for over a week.

[rhettl]

Okay, I think I finally have an idea of where my issue may be coming from, but still no idea how to fix it. I think my issue may be stemming from app.render and res.render loading different View objects.

In express/lib/application.js in app.render found here there is a segment which seems to be the issue.

var View = this.get('view');

When using res.render the View object is from engine-munger/index.js located here

When using app.render the View object is from express/lib/view.js located here.

The problem is when the program eventually gets to dust-makara-helpers/index.js line 37, which it does in both cases, ctx.options.view.lookup which refers to the function View.prototype.lookup is trying to pass 3 arguments: bundle, { locale: locale }, and a callback. Since the express/lib/view.js View.prototype.lookup only takes one argument, the { locale: locale } and callback are getting lost, making for a silent failure.

Any ideas how I might make the View object in app.render be the View object created in engine-munger/index.js? This seems like a big problem to me. At the very least the fact that it is failing silently must be of concern to someone, right?

[grawk]

Hi @rhettl I'd like to help you with this. Do you think you can create a sample application that I can run which illustrates this issue?

[rhettl]

Yes, I'll make one today and post it publicly.
Good suggestion.

[rhettl]

Hello @grawk
Here is the repo. /rhettl/kraken-app-render
I have added my email.js and emailController.js just to give the bigger picture, but there is a function in index.js which will run the app.renderdirectly.

As proof that the email template is rendering successfully, I added a /testemail route to render that same template, emails/test, through res.render.

Tell me if there is anything else I can do to help in this debugging process

[grawk]

I think the short answer to the issue is that the "app" instance you have outside of the request lifecycle:
a) is using the adaro dust engine but
b) does not have the engine-munger "view render" method

I'm not sure whether it's beneficial or not to make sure that the original app instance satisfies "b" or not. But I identified a fairly quick workaround, which is to use the "app" instance you can pull from the "req" object in the route handler to render the emails instead:

  router.get('/testemail', function (req, res, next) {

    model.subject = 'Test email';
    //res.render('emails/test', model);
    req.app.render('emails/test', {
      subject: 'test subject'
    }, function(err, markup){
      console.log('... Email sent with:');
      if (err) {
        console.error(err.stack);
        return next(err);
      } else {
        console.log(markup);
        res.render('emails/test', model);
      }
    });
  });

[rhettl]

I know that this would definitely work, but I don't have an incoming request. I added the /testemail route just to show that my template was rendering successfully and that all worked as expected when using res.render.

What I need is to send emails to users based on a time schedule. So there is no Request object or Response object. My present scheme uses an initialization function called from ./index.js L33 to initialize a setTimeout. Each morning, the application will run a query to the database, send emails to people returned by that query, and set the setTimeout for the next day.

If there is no way to make app.render operate similar to res.render then is there a way to get a Request or Response object without an HTTP call?

I know I can make an HTTP GET call using a tool like request, but that means I will be sending a new HTTP GET to localhost just to call a route, which I also don't want to be called from an external machine. To me writing a function and giving it a route, just to be called by the same server seems like bad form.

In lieu of being able to use app.render, convincing express to give me a fully formed Request or Response object, which has all the kraken/adaro/makara/dust utilities, would be the next best thing.

[aredridel]

Yeah, mocking request and response is a lesson in pain. Don't go that route.

have you looked at Bundalo, or the getBundle method of makara 2?

[grawk]

Good point @aredridel. You can use bundalo to resolve the localized content bundle(s), do whatever other machinations to the data model, and then use dust directly to render the email. There is no need to use the express application at all in this scenario.

[aredridel]

Indeed. No need to get express tangled up in your emails.

[grawk]

This use case seemingly solved, it still is somewhat troubling that app.render is rendered useless... Unless we add something to the documentation regarding its usage as an anti pattern.

[aredridel]

Indeed. That's worth figuring out and fixing up.

[aredridel]

Which versions of things are being used here?

[grawk]

"dependencies": {
    "construx": "^1.0.0",
    "construx-copier": "^1.0.0",
    "construx-dustjs": "^1.1.0",
    "construx-sass": "^1.0.0",
    "dust-makara-helpers": "^4.1.2",
    "express": "^4.12.2",
    "extend": "^3.0.0",
    "kraken-js": "^1.0.3",
    "makara": "^2.0.3",
    "nodemailer": "^1.8.0",
    "nodemailer-sendmail-transport": "^1.0.0"
  },

[rhettl]

It looks like Makara's getBundler still requires a Request object. From makara/index.js#L88

function getBundler(req) {
    var bundler = associated.get(req);
    if (!bundler) {
        throw new Error("No bundle reader available");
    } else {
        return bundler;
    }
}

I'm looking at Makara index.js to figure out how to use Bundalo on my own. Will I need to require Engine-Munger as well?

If I only need Bundalo, I can use:

var options = {} // from config.js
var bundler = bundalo({
  contentPath: options.i18n.contentPath 
});
/* ... */
bundler.get({bundle: bundle, locality: options.i18n.fallback, model: model}, cb);

In my previous debugging it looked like the Engine-Munger View object would be important and I now see that it is rather easy, some basic options, .properties, .js, .dust and the call the function returned from require('engine-munger');. From this file, I don't see where they actually meet. Will I need this View object or will it be created automatically?

[aredridel]

engine-munger is entirely for Express. It shouldn't be needed.

[grawk]

^^^ you can obtain the correct properties file(s) using bundalo (which will return them as a JSON structure) and then use dustjs-linkedin directly to perform the email rendering.

[rhettl]

I think I understand. I am going to try this now.

[rhettl]

Also, finding a way to place a warning or error message in the app.render would probably be nice. As it is the function silently dies.

[grawk]

Agreed. I've started looking into that aspect.

[aredridel]

Or making it work :)

[rhettl]

@aredridel

No need to get express tangled up in your emails.

I think it makes sense to use app.render to render your emails. It feel natural to have app.render and res.render produce the same result and then they only need to rely on one configuration, the same helpers, the same i18n, etc.

While It is obviously ideal for large systems to separate mail sending to another server and handle it away from where the users interact, in small projects it is easier to just let express render the content for you.

[rhettl]

In my real project, I have additional dust helpers.

{
/* ... */
  "view engines": {
    "js": {
      "module": "makara",
      "renderer": {
        "method": "js",
        "arguments": [
          {
            "cache": true,
            "helpers": "config:dust.helpers"
          }
        ]
      }
    }
  },
  "dust": {
    "helpers": [
      "dust-makara-helpers",
      "./lib/dust-helpers"
    ]
  }
/* ... */
}

I'm trying to find a point where I can grab the already configured Dust so I don't have to re-include the dust-makara-helpers and ./lib/dust-helpers

[aredridel]

My advice there: don't. Stealing the instance is just asking for trouble. That said, I think I left a .dust property dangling out where you can find it.

[rhettl]

@aredridel, I think I found it under app.engines['.dust']

I'm interested to hear why it is asking for trouble. The first thing that comes to mind is kraken changing something and having the emails break because of it, but then the same would happen to my web templates as well.

[aredridel]

It's just a lot of state sharing.

[aredridel]

krakenjs/makara#79 is my start at showing app.render working. Just needs locale info supplied.

[rhettl]

👍

[rhettl]

So after some experimenting I think the final reason app.render gets the wrong View object in my example is because of this excerpt from makara/index.js

var hasConfiguredApp = false;
return function (req, res, next) {
    if (!hasConfiguredApp) {
        req.app.set('view', makeViewClass(opts));
        hasConfiguredApp = true;
    }

I have tried moving my email.init() function to a later stage and found that all of the settings are right to render properly, but app.get('view') produces the wrong object. The issue is that makeViewClass, which refers to engine-munger only gets called on the first request. If it were called during the initialization stages, I think the app.render would work as expected.

[rhettl]

The easiest way I found to make this work involved duplicating much of makara/index.js. Here is my working result, I will implement the changes in my actual project tomorrow.

Note in my ./lib/email.js I ended up requiring makara and engine-munger. Then got a View object from engine-munger the way makara does. The View object's render method already includes the bundalo and dust fetching, normalizing, and rendering functionality.

I'm probably going to streamline this as best I can and make it a module later in the week.

[aredridel]

Good luck!

The reason it's called at first init is a bit arcane, and tied into Kraken specifically. I'd totally be into adding an "initialize now" method for non-kraken use to makara.

[rhettl]

You know, that sounds like a great idea. 👍

[rhettl]

For the sake of future people with this problem, my new render function looks like this

var extend            = require('extend');
var makara            = require('makara');
var engineMunger      = require('engine-munger');

var baseTemplateDefaults = {};
module.exports = {
  __app             : null,
  __view            : null,
  __cache        : {},
  __templateDefaults: {},
  /**
   * Initialization - Called from app.on('start');
   * @param app  {object} kraken/express application
   * @param conf {object} configuration settings from config.js; either app.kraken or the confit object
   */
  config            : function (app, conf) {
    var mailOpts = conf.get('email');
    var i18nOpts = conf.get('i18n');
    var specOpts = conf.get('specialization');

    module.exports.__app              = app;
    module.exports.__view             = this.__makeView(i18nOpts, specOpts);
    module.exports.__templateDefaults = extend({}, baseTemplateDefaults, mailOpts.templateDefaults || {});
  },
  __makeView        : function (i18n, specialization) {
    var opts            = {};
    opts['.properties'] = {};
    opts['.js']         = {};
    opts['.dust']       = {};


    if (i18n) {
      opts['.properties'].root = [].concat(i18n.contentPath);
      opts['.properties'].i18n = {
        formatPath: i18n.formatPath || makara.formatPath,
        fallback  : i18n.fallback
      };
    }

    if (specialization) {
      opts['.properties'].specialization = specialization;
      opts['.js'].specialization         = specialization;
      opts['.dust'].specialization       = specialization;
    }
    return engineMunger(opts);
  },

  __newView: function (name) {


    var view = new module.exports.__view(name, {
      defaultEngine: module.exports.__app.get('view engine'),
      root         : module.exports.__app.get('views'),
      engines      : module.exports.__app.engines
    });

    if (!view.path) {
      var dirs = Array.isArray(view.root) && view.root.length > 1
        ? 'directories "' + view.root.slice(0, -1).join('", "') + '" or "' + view.root[view.root.length - 1] + '"'
        : 'directory "' + view.root + '"';
      var err  = new Error('Failed to lookup view "' + name + '" in views ' + dirs);
      err.view = view;
      throw err;
    }

    return view;
  },
  render: function (template, data, callback) {
    var cache    = module.exports.__cache;
    var tempData = extend({}, module.exports.__templateDefaults, data);
    var name     = 'emails/' + template;
    var view;

    if (tempData.cache == null) {
      tempData.cache = module.exports.__app.enabled('view cache');
    }
    if (tempData.cache) {
      view = cache[name];
    }

    try {
      view = view || this.__newView(name);

      // prime the cache
      if (tempData.cache) {
        cache[name] = view;
      }

      view.render(tempData, callback);
    } catch (err) {
      callback(err);
    }
  },
}

As I think you can see, __makeView, __newView, and much of render are basically copied from makara and express's application.js

[aredridel]

@rhettl can you try updating to adaro@1.0.1 ? That may fix your issue.

[rhettl]

@aredridel

I moved my work to a "fixed" branch and reset "master" to an earlier commit which still had the problem.

The application doesn't immediately depend on adaro. I went to node_modules/makara/package.json which is makara@2.0.3 and change its adaro@1.0.0 to adaro@1.0.1. Then I did an npm install on the node_modules/makara directory.

On running the code, it still fails identically. Was this the workflow you wanted? Should I have tried something differently?

[aredridel]

That sounds about right (npm update I think would have done it.)

Interesting that it fails silently. I added a test case to makara that shows content being rendered using app.render in #79 and it now passes with that change.

[rhettl]

I see, you changed the test too, so that it is using a mock req and res to init the engine-munger instance

[aredridel]

Oh, yes, yes I did. I'd be happy to expose a cleaner initializer in makara for use where requests have not run yet. The way it's built is wacky 100% to appease Kraken and its config language which is limited in this case.