Skip to content

Commit

Permalink
http2: respondWith* to the new error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
mcollina committed Aug 27, 2017
1 parent 054c9c6 commit 2857f15
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 3 deletions.
16 changes: 16 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,8 @@ added: v8.4.0
* `headers` {[Headers Object][]}
* `options` {Object}
* `statCheck` {Function}
* `onError` {Function} Callback function invoked in the case of an
Error before send
* `getTrailers` {Function} Callback function invoked to collect trailer
headers.
* `offset` {number} The offset position at which to begin reading
Expand Down Expand Up @@ -1146,6 +1148,16 @@ server.on('stream', (stream) => {
function statCheck(stat, headers) {
headers['last-modified'] = stat.mtime.toUTCString();
}

function onError(err) {
if (err.code === 'ENOENT') {
stream.respond({ ':status': 404 });
} else {
stream.respond({ ':status': 500 });
}
stream.end();
}

stream.respondWithFile('/some/file',
{ 'content-type': 'text/plain' },
{ statCheck });
Expand Down Expand Up @@ -1178,6 +1190,10 @@ The `offset` and `length` options may be used to limit the response to a
specific range subset. This can be used, for instance, to support HTTP Range
requests.

The `options.onError` function may also be used to handle all the errors
that could happen before the delivery of the file is initiated. The
default behavior is to destroy the stream.

When set, the `options.getTrailers()` function is called immediately after
queuing the last chunk of payload data to be sent. The callback is passed a
single object (with a `null` prototype) that the listener may used to specify
Expand Down
22 changes: 19 additions & 3 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1640,13 +1640,24 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
abort(this);
return;
}
const onError = options.onError;

if (err) {
process.nextTick(() => this.emit('error', err));
if (onError) {
onError(err);
} else {
this.destroy(err);
}
return;
}

if (!stat.isFile()) {
err = new errors.Error('ERR_HTTP2_SEND_FILE');
process.nextTick(() => this.emit('error', err));
if (onError) {
onError(err);
} else {
this.destroy(err);
}
return;
}

Expand Down Expand Up @@ -1683,12 +1694,17 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {

function afterOpen(session, options, headers, getTrailers, err, fd) {
const state = this[kState];
const onError = options.onError;
if (this.destroyed || session.destroyed) {
abort(this);
return;
}
if (err) {
process.nextTick(() => this.emit('error', err));
if (onError) {
onError(err);
} else {
this.destroy(err);
}
return;
}
state.fd = fd;
Expand Down
48 changes: 48 additions & 0 deletions test/parallel/test-http2-respond-file-404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const assert = require('assert');
const path = require('path');

const {
HTTP2_HEADER_CONTENT_TYPE
} = http2.constants;

const server = http2.createServer();
server.on('stream', (stream) => {
const file = path.join(process.cwd(), 'not-a-file');
stream.respondWithFile(file, {
[HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}, {
onError(err) {
common.expectsError({
code: 'ENOENT',
type: Error,
message: `ENOENT: no such file or directory, open '${file}'`
})(err);

stream.respond({ ':status': 404 });
stream.end();
},
statCheck: common.mustNotCall()
});
});
server.listen(0, () => {

const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();

req.on('response', common.mustCall((headers) => {
assert.strictEqual(headers[':status'], 404);
}));
req.on('data', common.mustNotCall());
req.on('end', common.mustCall(() => {
client.destroy();
server.close();
}));
req.end();
});
46 changes: 46 additions & 0 deletions test/parallel/test-http2-respond-file-error-dir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const assert = require('assert');

const {
HTTP2_HEADER_CONTENT_TYPE
} = http2.constants;

const server = http2.createServer();
server.on('stream', (stream) => {
stream.respondWithFile(process.cwd(), {
[HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}, {
onError(err) {
common.expectsError({
code: 'ERR_HTTP2_SEND_FILE',
type: Error,
message: 'Only regular files can be sent'
})(err);

stream.respond({ ':status': 404 });
stream.end();
},
statCheck: common.mustNotCall()
});
});
server.listen(0, () => {

const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();

req.on('response', common.mustCall((headers) => {
assert.strictEqual(headers[':status'], 404);
}));
req.on('data', common.mustNotCall());
req.on('end', common.mustCall(() => {
client.destroy();
server.close();
}));
req.end();
});

0 comments on commit 2857f15

Please sign in to comment.