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

process.domain is undefined after using "await" in node7 #10724

Closed
zkd8907 opened this issue Jan 10, 2017 · 8 comments
Closed

process.domain is undefined after using "await" in node7 #10724

zkd8907 opened this issue Jan 10, 2017 · 8 comments
Labels

Comments

@zkd8907
Copy link

@zkd8907 zkd8907 commented Jan 10, 2017

  • Version: found at v7.4.0, reproducible at v7.2.0
  • Platform: 64-bit Windows 10, Darwin Kernel Version 16.3.0(macOS 10.12.2)
  • Subsystem:domain

I am trying to transform my code from Promise to await/async in node7, but I found it didn't work with domain.
Here is my code:

let d = require('domain').create();

d.run(async function(){
    console.log(d === process.domain);
    // true
    console.log(1, process.domain);
    // process.domain exists

    let result = await (async() => {
        return new Promise((resolve) => {
            resolve('ok');
        });
    })();

    console.log(result, process.domain, d);
    /**
     * result: ok
     * process.domain: undefined
     * d: no change
     */
});

As shown by the comments, process.domain is undefined after using await. However, If I remove await the result will be a Promise and process.domain is the same with d.
Is is a bug or something undocumented.

@jasnell jasnell added the promises label Jan 10, 2017
@sam-github
Copy link
Member

@sam-github sam-github commented Jan 10, 2017

Not doced in https://github.com/nodejs/node/blob/master/doc/topics/domain-postmortem.md, however, domains are deprecated in the docs, for reasons such as this.

They are known to not work with promises, though I'm having trouble finding a ref.

nodejs/node-v0.x-archive#8648 discusses some issues

@vkurchatkin
Copy link
Contributor

@vkurchatkin vkurchatkin commented Jan 10, 2017

We can fix this once V8 5.5 lands

@zkd8907
Copy link
Author

@zkd8907 zkd8907 commented Jan 11, 2017

@sam-github
I've found the same situation when I am using Promise, that process.domain is undefined in callback. When I am using Promise, I hacked Promise.prototype.then and Promise.prototype.catch as the code blow:

Promise.prototype.then = function(orign){
        return function(){
                var d = process.domain;
                var f1 = arguments[0];
                var f2 = arguments[1];
                if(d && typeof f1 === 'function'){
                        arguments[0] = d.bind(f1);
                }
                if(d && typeof f2 === 'function'){
                        arguments[1] = d.bind(f2);
                }

                return orign.apply(this, arguments);
        }

process.domain exists in callback after the hack by myself. However, there is no way to hack await/async.

@addaleax
Copy link
Member

@addaleax addaleax commented Apr 21, 2017

This would be fixed by #12489

addaleax added a commit to addaleax/node that referenced this issue Apr 24, 2017
Fixes: nodejs#10724
@addaleax addaleax closed this in 84dabe8 Apr 27, 2017
@emilsedgh
Copy link

@emilsedgh emilsedgh commented May 8, 2017

Thank you so much for fixing this.

Is it gonna be released with 7.11?

@addaleax
Copy link
Member

@addaleax addaleax commented May 9, 2017

@emilsedgh Sorry, you’ll need to wait for 8.0.0 – older versions of V8 don’t have the necessary hooks for Node to handle this.

edit: but just so you know, we’ll be bringing out a 8.0.0 RC very shortly – please try that out!

@addaleax addaleax removed the v7.x label May 9, 2017
@emilsedgh
Copy link

@emilsedgh emilsedgh commented May 9, 2017

Oh thank you for letting me know. Fingers crossed for 8.0!
I tried it out by building master and it works perfectly fine.

@zkd8907
Copy link
Author

@zkd8907 zkd8907 commented Jun 1, 2017

Thanks for fixing the issus. It works well at 8.0.0 now.

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

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.