-
Notifications
You must be signed in to change notification settings - Fork 459
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
app.render silent fail #429
Comments
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 In var View = this.get('view'); When using When using The problem is when the program eventually gets to Any ideas how I might make the |
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? |
Yes, I'll make one today and post it publicly. |
Hello @grawk As proof that the email template is rendering successfully, I added a Tell me if there is anything else I can do to help in this debugging process |
I think the short answer to the issue is that the "app" instance you have outside of the request lifecycle: 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);
}
});
}); |
I know that this would definitely work, but I don't have an incoming request. I added the What I need is to send emails to users based on a time schedule. So there is no If there is no way to make 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 In lieu of being able to use |
Yeah, mocking request and response is a lesson in pain. Don't go that route. have you looked at Bundalo, or the |
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. |
Indeed. No need to get express tangled up in your emails. |
This use case seemingly solved, it still is somewhat troubling that |
Indeed. That's worth figuring out and fixing up. |
Which versions of things are being used here? |
"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"
}, |
It looks like Makara's 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 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 |
engine-munger is entirely for Express. It shouldn't be needed. |
^^^ you can obtain the correct properties file(s) using |
I think I understand. I am going to try this now. |
Also, finding a way to place a warning or error message in the |
Agreed. I've started looking into that aspect. |
Or making it work :) |
I think it makes sense to use 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. |
In my real project, I have additional 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 |
My advice there: don't. Stealing the instance is just asking for trouble. That said, I think I left a |
@aredridel, I think I found it under 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. |
It's just a lot of state sharing. |
krakenjs/makara#79 is my start at showing app.render working. Just needs locale info supplied. |
👍 |
So after some experimenting I think the final reason var hasConfiguredApp = false;
return function (req, res, next) {
if (!hasConfiguredApp) {
req.app.set('view', makeViewClass(opts));
hasConfiguredApp = true;
} I have tried moving my |
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 I'm probably going to streamline this as best I can and make it a module later in the week. |
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. |
You know, that sounds like a great idea. 👍 |
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, |
@rhettl can you try updating to |
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 On running the code, it still fails identically. Was this the workflow you wanted? Should I have tried something differently? |
That sounds about right ( 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. |
I see, you changed the test too, so that it is using a mock |
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. |
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 copyres.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 inlocaleFromContext
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
and a section of my
config.json
(with some minor modification of the personal information)Thank you for any help you might be able to provide. I have been racking my brain on this for over a week.
The text was updated successfully, but these errors were encountered: