Skip to content
This repository has been archived by the owner on Jul 6, 2018. It is now read-only.

Commit

Permalink
http2: refinement and test for socketError
Browse files Browse the repository at this point in the history
Fixes: #184

Refines the `'socketError'` event a bit and adds a test for the
emission of the `'socketError'` event on the server. Client side
is tested separately
  • Loading branch information
jasnell committed Jul 22, 2017
1 parent 78be06c commit 0281788
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 29 deletions.
16 changes: 7 additions & 9 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,9 @@ The `'socketError'` event is emitted when an `'error'` is emitted on the
`Socket` instance bound to the `Http2Session`. If this event is not handled,
the `'error'` event will be re-emitted on the `Socket`.

Likewise, when an `'error'` event is emitted on the `Http2Session`, a
`'sessionError'` event will be emitted on the `Socket`. If that event is
not handled, the `'error'` event will be re-emitted on the `Http2Session`.
For `ServerHttp2Session` instances, a `'socketError'` event listener is always
registered that will, by default, forward the event on to the owning
`Http2Server` instance if no additional handlers are registered.

#### Event: 'timeout'
<!-- YAML
Expand Down Expand Up @@ -1117,9 +1117,8 @@ an `Http2Session` object. If no listener is registered for this event, an
added: REPLACEME
-->

The `'socketError'` event is emitted when an `'error'` event is emitted by
a `Socket` associated with the server. If no listener is registered for this
event, an `'error'` event is emitted.
The `'socketError'` event is emitted when a `'socketError'` event is emitted by
an `Http2Session` associated with the server.

#### Event: 'stream'
<!-- YAML
Expand Down Expand Up @@ -1181,9 +1180,8 @@ an `Http2Session` object. If no listener is registered for this event, an
added: REPLACEME
-->

The `'socketError'` event is emitted when an `'error'` event is emitted by
a `Socket` associated with the server. If no listener is registered for this
event, an `'error'` event is emitted on the `Socket` instance instead.
The `'socketError'` event is emitted when a `'socketError'` event is emitted by
an `Http2Session` associated with the server.

#### Event: 'unknownProtocol'
<!-- YAML
Expand Down
31 changes: 11 additions & 20 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -2032,17 +2032,15 @@ function sessionOnError(error) {
this.emit('error', error);
}

// When a Socket emits an error, first try to forward it to the server
// as a socketError; failing that, forward it to the session as a
// When a Socket emits an error, forward it to the session as a
// socketError; failing that, remove the listener and call destroy
function socketOnError(error) {
const type = this[kSession] && this[kSession][kType];
debug(`[${sessionName(type)}] server socket error: ${error.message}`);
if (kRenegTest.test(error.message))
return this.destroy();
if (this[kServer] !== undefined && this[kServer].emit('socketError', error))
return;
if (this[kSession] !== undefined && this[kSession].emit('socketError', error))
if (this[kSession] !== undefined &&
this[kSession].emit('socketError', error, this))
return;
this.removeListener('error', socketOnError);
this.destroy(error);
Expand Down Expand Up @@ -2076,6 +2074,11 @@ function sessionOnPriority(stream, parent, weight, exclusive) {
this[kServer].emit('priority', stream, parent, weight, exclusive);
}

function sessionOnSocketError(error, socket) {
if (this.listenerCount('socketError') <= 1 && this[kServer] !== undefined)
this[kServer].emit('socketError', error, socket, this);
}

function connectionListener(socket) {
debug('[server] received a connection');
const options = this[kOptions] || {};
Expand Down Expand Up @@ -2109,6 +2112,7 @@ function connectionListener(socket) {
session.on('error', sessionOnError);
session.on('stream', sessionOnStream);
session.on('priority', sessionOnPriority);
session.on('socketError', sessionOnSocketError);

socket[kServer] = this;

Expand Down Expand Up @@ -2193,27 +2197,14 @@ function setupCompat(ev) {
}
}

// If the socket emits an error, forward it to the session as a socketError;
// failing that, remove the listener and destroy the socket
function clientSocketOnError(error) {
const type = this[kSession] && this[kSession][kType];
debug(`[${sessionName(type)}] client socket error: ${error.message}`);
if (kRenegTest.test(error.message))
return this.destroy();
if (this[kSession] !== undefined && this[kSession].emit('socketError', error))
return;
this.removeListener('error', clientSocketOnError);
this.destroy(error);
}

// If the session emits an error, forward it to the socket as a sessionError;
// failing that, destroy the session, remove the listener and re-emit the error
function clientSessionOnError(error) {
debug(`client session error: ${error.message}`);
if (this[kSocket] !== undefined && this[kSocket].emit('sessionError', error))
return;
this.destroy();
this.removeListener('error', clientSocketOnError);
this.removeListener('error', socketOnError);
this.removeListener('error', clientSessionOnError);
}

Expand Down Expand Up @@ -2249,7 +2240,7 @@ function connect(authority, options, listener) {
throw new errors.Error('ERR_HTTP2_UNSUPPORTED_PROTOCOL', protocol);
}

socket.on('error', clientSocketOnError);
socket.on('error', socketOnError);
socket.on('resume', socketOnResume);
socket.on('pause', socketOnPause);
socket.on('drain', socketOnDrain);
Expand Down
52 changes: 52 additions & 0 deletions test/parallel/test-http2-server-socketerror.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
const assert = require('assert');
const http2 = require('http2');

const server = http2.createServer();
server.on('stream', common.mustCall((stream) => {
stream.respond();
stream.end('ok');
}));
server.on('session', common.mustCall((session) => {
// First, test that the socketError event is forwarded to the session object
// and not the server object.
const handler = common.mustCall((error, socket) => {
common.expectsError({
type: Error,
message: 'test'
})(error);
assert.strictEqual(socket, session.socket);
});
const isNotCalled = common.mustNotCall();
session.on('socketError', handler);
server.on('socketError', isNotCalled);
session.socket.emit('error', new Error('test'));
session.removeListener('socketError', handler);
server.removeListener('socketError', isNotCalled);

// Second, test that the socketError is forwarded to the server object when
// no socketError listener is registered for the session
server.on('socketError', common.mustCall((error, socket, session) => {
common.expectsError({
type: Error,
message: 'test'
})(error);
assert.strictEqual(socket, session.socket);
assert.strictEqual(session, session);
}));
session.socket.emit('error', new Error('test'));
}));

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();
req.resume();
req.on('end', common.mustCall());
req.on('streamClosed', common.mustCall(() => {
client.destroy();
server.close();
}));
}));

0 comments on commit 0281788

Please sign in to comment.