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

Promise in core-js doesn't work with domain in nodejs #103

Closed
jiaz opened this issue Aug 28, 2015 · 4 comments
Closed

Promise in core-js doesn't work with domain in nodejs #103

jiaz opened this issue Aug 28, 2015 · 4 comments

Comments

@jiaz
Copy link

jiaz commented Aug 28, 2015

I use domain in nodejs to store data related to an async chain. Below is the code to demonstrate the concept.

in index.js

require('babel/register');
require('./server');

in server.js

import express from 'express';
import domain from 'domain';
import crypto from 'crypto';

const app = express();

app.use((req, res, next) => {
  const requestDomain = domain.create();
  requestDomain.add(req);
  requestDomain.add(res);
  requestDomain.trackingId = crypto.randomBytes(16).toString('hex');
  requestDomain.run(() => {
    next();
  });
});

app.get('/user/:id', (req, res) => {
  const trackingId = process.domain.trackingId;
  new Promise((resolve) => {
    setTimeout(resolve, 1000);
  }).then(() => {
    if (process.domain.trackingId !== trackingId) {
      throw new Error('ahhh, domain messed up!');
    }
    res.status(200).json({
      user: req.params.id,
    });
  }).catch((err) => {
    console.log(err.message);
    res.status(500).json(err.stack);
  });
});

app.listen(3000, () => {
  console.log('server started');
});

So the idea is when a request is being handled by express, a trackingId will be generated and bound to a domain scoped to the current request. According to nodejs' documentation process.domain will then always refer to that domain object during the whole async process chain.

In the code I added a check to verify that the trackingId is the same after the promise is resolved. However when I use the Promise provided by babel (core-js implementation) the Error will be thrown. So actually, somehow the Promise breaks the domain scope.

I then replace the Promise implementation with the bluebird one, everything then works well. Essentially, I changed the index.js to

require('babel/register');
global.Promise = require('bluebird');
require('./server');

So it looks like the Promise in core-js has some problem with nodejs' domain. Although I currently have a workaround on this, I think it might be good if someone can look at it.

@zloirock
Copy link
Owner

Problems with domains probable, but I can't reproduce it with your example - process.domain.trackingId === trackingId in io.js with replaced Promise. Please, write simple reproduceble example or change current and I will fix it.

@jiaz
Copy link
Author

jiaz commented Aug 28, 2015

Hi @zloirock ,
Thanks for looking into this. Sorry that the description is not clear enough.
The example will reproduce when you use a concurrent client to stress it.

Below, I give a self-contained test file.

var domain = require('domain');
var core = require('core-js/library');
var crypto = require('crypto');
var Promise = require('bluebird');

console.log(process.version);

function runInDomain(fn) {
    var d = domain.create();
    d.run(fn);
}

var counter = 0;

function test() {
    runInDomain(function() {
        process.domain.trackingId = crypto.randomBytes(16).toString('hex');
        var trackingId = process.domain.trackingId;
        console.log('>' + trackingId);
        // NOTE: replace core.Promise with Promise, the error will disappear.
        new core.Promise(function(resolve) {
            setTimeout(resolve, 100);
        }).then(function() {
            console.log('<' + process.domain.trackingId);
            if (trackingId !== process.domain.trackingId) {
                console.log('error!');
            }
            counter++;
        });
    });
}

var loop = 5;

for (var i = 0; i < loop; ++i) {
    test();
}

setTimeout(function() {
    if (counter !== loop) {
        console.log('error, count not match');
    }
    console.log(counter);
}, 1000);

I tested it with node v0.10, 0.12, iojs 3.2.0, all show the same result. One example below.

$ node test
v0.10.39
>49888ebe680e1cdfe8825adc7d0055ae
>738516ee1d0dba70e3d72e2e8adf698b
>43c6f69e9540290ba0845c8af6927b61
>0053bf12104b660b946cd9b1e84e7b0f
>19c2f270905533ccebd0cfefe20d3805
<49888ebe680e1cdfe8825adc7d0055ae
<49888ebe680e1cdfe8825adc7d0055ae
error!
<49888ebe680e1cdfe8825adc7d0055ae
error!
<49888ebe680e1cdfe8825adc7d0055ae
error!
<49888ebe680e1cdfe8825adc7d0055ae
error!
5

Result after I replace with bluebird Promise

$ node test
v0.10.39
>980ef244549447a874b4b62b45fe3f1b
>19006bc9b4872d950957e41fcb592a92
>d3d536bf058dcb68e82fef1cbf136d64
>fb8d3c8fba1cfc55e7989a258e3706a9
>5b56f5823009500b860e9d5f15004ff3
<980ef244549447a874b4b62b45fe3f1b
<19006bc9b4872d950957e41fcb592a92
<d3d536bf058dcb68e82fef1cbf136d64
<fb8d3c8fba1cfc55e7989a258e3706a9
<5b56f5823009500b860e9d5f15004ff3
5

@zloirock
Copy link
Owner

Thanks! This commit should fix current problem. IIRC with domains possible another problems, any ideas about it?

@jiaz
Copy link
Author

jiaz commented Aug 28, 2015

The fix works great! Thanks!
I don't have more cases that are broken due to domains. I'll create issue here if I found more problems.

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

2 participants