Njs: new Promise() not working in nginx context

[tomachristian]

I am on njs version 0.4.4, nginx version 1.18.0. I am in the context of an js_content served by a function.

In that function I am doing a new Promise((resolve) => resolve(true)).then(result => request.error(result)); but when accessing the page served by this function it never returns, it just hangs. There is no output in the error.log with the result. It seems that new Promise() and Promise.resolve() are plain broken. When I do request.subrequest(...).then(...) the then is executed and all works fine.

What am I doing wrong or missing? Is the Promise constructor broken, do only promises returned by built-in functions work?

[tomachristian]

I just checked if there is a difference between synchronously completing a Promise or asynchronously completing it (something to be scheduled on the event-loop). So:

It seems that this does not work:

request.error("1");
Promise.resolve(true).then(function (authenticated) {
    request.error("2");
});
request.error("3");

prints 1 and 3

but, this works:

request.error("1");
new Promise(resolve => setTimeout(resolve, 1)).then(function (authenticated) {
    request.error("2");
});
request.error("3");

prints 1, 2 and 3.

This does not seem to be happening in the njs CLI, only during an HTTP request handled by js_content.

[lexborisov]

Hi @tomachristian

You found a bug! Try this patch to fix the problem.
After thorough testing, I will commit the patch (most likely tomorrow morning).

Thanks for the report!

[jirutka]

I encountered this problem just yesterday, but I thought that’s maybe how it’s supposed to work. I’ve used setImmediate to make it work.

request.error("1");
var promise = Promise.resolve(true).then(function (authenticated) {
    request.error("2");
});
setImmediate(promise);
request.error("3");

[lexborisov]

@jirutka

I encountered this problem just yesterday, but I thought that’s maybe how it’s supposed to work. I’ve used setImmediate to make it work.

Just setImmediate started processing post events, and because of this, promises were processed. But with the patch they will work out without these tweaks.

CLI doesn't have this problem, only nginx.

[tomachristian]

@lexborisov I'm on v0.4.4, so my sources do not match; I cannot apply the patch to test it myself. do you have (want to provide) a patch of 0.4.4? I can try and figure out how to do it myself, but its the second day for me seeing the nginx/njs code base :) so..... its gonna take me a while.

PS: I've seen those nice tests you guys have, I think it would be good to somehow also run those in nginx context, not only njs CLI.

[xeioex]

@tomachristian tests like this https://github.com/nginx/nginx-tests/blob/master/js_promise.t are executed only in nginx context

[tomachristian]

@xeioex I meant these https://github.com/nginx/njs/blob/master/test/js/promise_s2.js

[lexborisov]

@xeioex I meant these https://github.com/nginx/njs/blob/master/test/js/promise_s2.js

@tomachristian this tests for CLI.

[tomachristian]

I know, I am saying that it would be good to have them for NGINX as well. To catch these sort of problems like the one described here.