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

throwing inside an event breaks new events from emitting #18

Closed
aalimovs opened this issue Mar 24, 2017 · 5 comments
Closed

throwing inside an event breaks new events from emitting #18

aalimovs opened this issue Mar 24, 2017 · 5 comments
Assignees
Labels
Milestone

Comments

@aalimovs
Copy link

@aalimovs aalimovs commented Mar 24, 2017

Original issue in hapi: hapijs/hapi#3464

In short, there's a difference how podium and node's EventEmitter handle errors inside events. Currently, once thrown inside a podium event, it'll stop emitting new events. Where as node's EventEmitter does not break future events from being triggered.

I hunted this down in production hapi v16, where all of my plugins that rely on the response event stopped working, while the server carried on serving requests, but with an increased memory footprint.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Mar 24, 2017

This is probably a bug but why are you throwing? Could you not just log the error?

@aalimovs

This comment has been minimized.

Copy link
Author

@aalimovs aalimovs commented Mar 24, 2017

I didn't pinpoint yet which of the plugins listening to response events throws, I don't see anything in stdout/stderr.

I've got node running across 3 instances on AWS Elastic Beanstalk. Randomly, either one of the 3 or all 3 together at the same time stop emitting response events, because my logger and metrics plugins that rely on the response event stop sending anything new. I can still see ops logs and metrics coming through from the same server, but nothing response related.

I also notice server that stopped emitting response events has a constantly growing memory usage from the minute I stopped receiving response logs and metrics compared to other servers.

I've tried to hunt the bug locally and encountered this issue.

@aalimovs

This comment has been minimized.

Copy link
Author

@aalimovs aalimovs commented Mar 24, 2017

Just to add, it seems it breaks all events from being emitted in hapi, not just response. Not getting either request-internal or request events.

@hueniverse hueniverse self-assigned this May 27, 2017
@hueniverse hueniverse added the bug label May 27, 2017
@hueniverse hueniverse added this to the 1.2.6 milestone May 27, 2017
@hueniverse hueniverse removed the bug label May 27, 2017
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented May 27, 2017

@aalimovs Node's event emitter doesn't catch errors thrown in event handlers. I am not sure what you expect to happen when your code throws. Can you write a simple example showing both podium and node handling of such an error?

@aalimovs

This comment has been minimized.

Copy link
Author

@aalimovs aalimovs commented Jul 5, 2017

@hueniverse, here's an example:

var Hapi = require('hapi');
var server = new Hapi.Server({});

server.connection({ port: 3000 });

server.start((err) => {
    if (err) { throw err }
    console.log('Server started');
});

server.on('response', function (request) {
    console.log('response event');
    foo.toString();
});

server.on('request-internal', function (request, event, tags) {
    console.log('request-internal event', tags);
    if (tags.internal) {
        console.log('internal error');
    }
});

Hitting http://localhost:3000 with hapi@14, will output:

Server started
request-internal event { received: true }
request-internal event { handler: true, error: true }
request-internal event { response: true }
response event
request-internal event { internal: true, implementation: true, error: true }
internal error
Debug: internal, implementation, error
    ReferenceError: foo is not defined
    at .<anonymous> (/Users/aalimovs/foo/server:13:5)
    at emitOne (events.js:96:13)
    at emit (events.js:188:7)
    at handler (/Users/aalimovs/foo/node_modules/kilt/lib/index.js:56:44)
    at emitOne (events.js:96:13)
    at emit (events.js:188:7)
    at internals.Request._finalize (/Users/aalimovs/foo/node_modules/hapi/lib/request.js:471:21)
    at Transmit.send (/Users/aalimovs/foo/node_modules/hapi/lib/request.js:447:25)
    at ServerResponse.end (/Users/aalimovs/foo/node_modules/hapi/lib/transmit.js:278:16)
    at ServerResponse.g (events.js:292:16)

Hitting the same with hapi@16:

Server started
request-internal event { received: true }
request-internal event { handler: true, error: true }
request-internal event { response: true }
response event
Debug: internal, implementation, error
    ReferenceError: foo is not defined
    at /Users/aalimovs/foo/server:13:5
    at invoke (/Users/aalimovs/foo/node_modules/podium/lib/index.js:239:30)
    at each (/Users/aalimovs/foo/node_modules/podium/lib/index.js:243:13)
    at Object.exports.parallel (/Users/aalimovs/foo/node_modules/items/lib/index.js:70:13)
    at Object.internals.emit (/Users/aalimovs/foo/node_modules/podium/lib/index.js:260:18)
    at internals.Podium._emit (/Users/aalimovs/foo/node_modules/podium/lib/index.js:140:15)
    at each (/Users/aalimovs/foo/node_modules/podium/lib/index.js:181:47)
    at Object.exports.parallel (/Users/aalimovs/foo/node_modules/items/lib/index.js:70:13)
    at relay (/Users/aalimovs/foo/node_modules/podium/lib/index.js:182:15)
    at Object.internals.emit (/Users/aalimovs/foo/node_modules/podium/lib/index.js:186:16)

Notice the missing

request-internal event { internal: true, implementation: true, error: true }
internal error

from hapi@16 after the response event.

Hitting hapi@14 again and again will continue outputting console.logs and the error, where as hapi@16 stops event processing, thus no new console.logs are printed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.