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

thrown errors inside server.inject does not propagate #1299

Closed
Starefossen opened this issue Jan 6, 2014 · 10 comments
Closed

thrown errors inside server.inject does not propagate #1299

Starefossen opened this issue Jan 6, 2014 · 10 comments
Assignees
Labels
bug Bug or defect
Milestone

Comments

@Starefossen
Copy link

The thrown error never appears in the console.

var server = require('hapi').createServer('localhost', 8000);
server.route({
  method: 'GET',
  path: '/',
  handler: function (request, reply) {
    reply('hello world');
  }
});

server.inject({method: 'GET', url: '/'}, function (res) {
  throw new Error('Never shown')
  console.log(res.result)
})
@hueniverse
Copy link
Contributor

I don't know what is the state of mocha support for node domains. hapi uses a domains a lot. This was the main reason we moved away from mocha and are using lab instead. If you look at the hapi tests, we use chai which throws inside server.inject() all the time.

@Starefossen
Copy link
Author

No, don't close this issue! It has nothing to do with Mocha and if you take a second look at the example code I provided you will see that it does not contain any Mocah related code – it is pure Hapi. Thrown errors does not proppogate from inside server.inject; but this might be an issue with shot instead of hapi?

@hueniverse hueniverse reopened this Jan 7, 2014
@hueniverse
Copy link
Contributor

This is odd. I can see the error if I wrap it in a domain:

var Hapi = require('hapi');
var Domain = require('domain');


var domain = Domain.createDomain();
domain.run(function () {

    var server = new Hapi.Server(8000);
    server.route({
        method: 'GET',
        path: '/',
        handler: function (request, reply) {

            reply('hello world');
        }
    });

    server.inject({ method: 'GET', url: '/' }, function (res) {

        throw new Error('Never shown');
        console.log(res.result);
    });
});

domain.on('error', function (err) {

    console.log(err.stack);
});

@dominykas
Copy link
Contributor

@Starefossen what other modules do you use in your system? I've just narrowed down some issues to either Q, or buster, or referree (buster's assertion library). Or my incorrect usage of these - not sure yet.

Disregard that. The initial code clearly shows there is also a problem in hapi, as well as elsewhere.

@bendrucker
Copy link
Contributor

To save people some searching, this is a mocha issue (and probably one w/ most other JS test frameworks). Mocha wraps all tests in a try/catch block which is what's causing this issue. The try/catch swallows the synchronous exceptions from assertions and those exceptions never makes it up to the domain's error handler. The try/catch is helpful for browser support, but with domains and uncaughtException handlers in Node there's no reason for it. You can fork Mocha and patch runnable and runner or the equivalents for Buster et al. Or use Lab.

Further reading: mochajs/mocha#513

@Starefossen
Copy link
Author

@bendrucker if you run the code snippet I have provided in the first post – or read my second comment – you can see that this error is reproducible without requiring any modules except for hapi itself. Hence, this is a hapi error.

@bendrucker
Copy link
Contributor

So you're expecting the thrown exception to emit an uncaughtException event on process? I originally came here having seen the issue with Mocha. I was assuming that you were just pulling the test code out of Mocha and interpreted "it has nothing to do with Mocha" to mean that you didn't think Mocha was the cause. In any case, just trying to be helpful.

@yonjah
Copy link

yonjah commented Nov 4, 2014

I came to this issue after looking for a way to use mocha with hapi.
From what I can see this is easy to do with Promises.
Too bad hapi doesn't use promises out of the box, but we could easily wrap the inject method to return a promise and write tests like so -

require('should');
var Promise = require("bluebird"),
    server = require('../');
function inject (options) {
    return new Promise(function (resolve, reject) {
        server.inject(options, resolve);
    });
}

describe('api', function(){
    it('should have this route', function () {
        return inject({ method: "GET", url: "/api/missing_route"}).then(function (response) {
            //this will throw an error
            response.statusCode.should.eql(200);
        });
    });
});

@bendrucker
Copy link
Contributor

@yonjah https://github.com/bendrucker/inject-then

@yonjah
Copy link

yonjah commented Nov 5, 2014

@bendrucker Cool. I guess it's probably better to have this as a reusable plugin then what I did.
I would have completely remove the builtin inject and make the server.inject method promiseable ( I'll send you a pull request).
But I would be much more happy if Hapi had promises build in since callbacks in general become messy quite quickly. Its weird nobody asked it before (couldn't find any issues) so I submitted this issue #2097

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

No branches or pull requests

5 participants