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

Regression on 16, lib/handlers.js #3399

Closed
simlevesque opened this issue Nov 30, 2016 · 7 comments
Closed

Regression on 16, lib/handlers.js #3399

simlevesque opened this issue Nov 30, 2016 · 7 comments
Assignees
Labels
Milestone

Comments

@simlevesque
Copy link
Contributor

@simlevesque simlevesque commented Nov 30, 2016

Hi everyone.

I'm experiencing a regression on 16. One of the routes on my hapi application won't respond. This route has a prerequisite without assignment and the error is triggered while still inside the pre.

The line that's causing problem is the 65th of lib/handler.js. The error says that domain is undefined, thus it cannot read it's bind property.

To fix the error, the only thing I did was change ^16.0.0 in my package.json to ^15.2.0, delete the node_modules and reinstall. Now everything works as it always did.

Sadly I can't give you access to my code and I don'T have the time at the moment to create a server which exhibits the bug. I'll try to do one soon.

Thank you.

@simlevesque simlevesque changed the title Regression on 16 Regression on 16, lib/handlers.js Nov 30, 2016
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Nov 30, 2016

Are you using promises?

@simlevesque
Copy link
Contributor Author

@simlevesque simlevesque commented Nov 30, 2016

I am. Let me try to do the same thing without them and see if the same thing happens.

@sirgallifrey
Copy link
Contributor

@sirgallifrey sirgallifrey commented Nov 30, 2016

This code works on 15.2.0 and fails in 16.0.0 in the smae way as @simlevesque described

const Hapi = require('hapi');

const server = new Hapi.Server({ useDomains: false });

server.connection({port: 3000});

server.route({
    method: 'get',
    path: '/',
    handler: function (req, reply) {

        reply('working');
    },
    config: {
        pre: [{
            method: function(req, reply) {
                
                reply('pre');
            }
        }]
    }
})

server.start((err) => {

    if (err) {
        throw err;
    }

    console.log('server running');
});

Note useDomains: false in combination with pre handler

@sirgallifrey
Copy link
Contributor

@sirgallifrey sirgallifrey commented Nov 30, 2016

Line 57 is just assuming there is a domain in place

@simlevesque
Copy link
Contributor Author

@simlevesque simlevesque commented Nov 30, 2016

NB: In my setup, useDomains is true.

@sirgallifrey
Copy link
Contributor

@sirgallifrey sirgallifrey commented Nov 30, 2016

@simlevesque I thought it would be. My guess is that there might be a lot o causes for process.domain === undefined I think we can focus on making handler.js know what to do when there is a domain and where there isn't

@hueniverse hueniverse added the bug label Nov 30, 2016
@hueniverse hueniverse added this to the 16.0.1 milestone Nov 30, 2016
@hueniverse hueniverse self-assigned this Nov 30, 2016
@sirgallifrey
Copy link
Contributor

@sirgallifrey sirgallifrey commented Dec 1, 2016

Noticed something that might be relevant, I'm trying to write a test for this, and I can't make a test that fails.... I don't have knowledge on how lab works, but I'm guessing it creates a domain and thus process.domain !== undefined

@hueniverse hueniverse closed this in 47a9566 Dec 1, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants