Skip to content

Commit 2857f15

Browse files
committed
http2: respondWith* to the new error handling
Fixes: #14963
1 parent 054c9c6 commit 2857f15

File tree

4 files changed

+129
-3
lines changed

4 files changed

+129
-3
lines changed

doc/api/http2.md

+16
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,8 @@ added: v8.4.0
11181118
* `headers` {[Headers Object][]}
11191119
* `options` {Object}
11201120
* `statCheck` {Function}
1121+
* `onError` {Function} Callback function invoked in the case of an
1122+
Error before send
11211123
* `getTrailers` {Function} Callback function invoked to collect trailer
11221124
headers.
11231125
* `offset` {number} The offset position at which to begin reading
@@ -1146,6 +1148,16 @@ server.on('stream', (stream) => {
11461148
function statCheck(stat, headers) {
11471149
headers['last-modified'] = stat.mtime.toUTCString();
11481150
}
1151+
1152+
function onError(err) {
1153+
if (err.code === 'ENOENT') {
1154+
stream.respond({ ':status': 404 });
1155+
} else {
1156+
stream.respond({ ':status': 500 });
1157+
}
1158+
stream.end();
1159+
}
1160+
11491161
stream.respondWithFile('/some/file',
11501162
{ 'content-type': 'text/plain' },
11511163
{ statCheck });
@@ -1178,6 +1190,10 @@ The `offset` and `length` options may be used to limit the response to a
11781190
specific range subset. This can be used, for instance, to support HTTP Range
11791191
requests.
11801192

1193+
The `options.onError` function may also be used to handle all the errors
1194+
that could happen before the delivery of the file is initiated. The
1195+
default behavior is to destroy the stream.
1196+
11811197
When set, the `options.getTrailers()` function is called immediately after
11821198
queuing the last chunk of payload data to be sent. The callback is passed a
11831199
single object (with a `null` prototype) that the listener may used to specify

lib/internal/http2/core.js

+19-3
Original file line numberDiff line numberDiff line change
@@ -1640,13 +1640,24 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
16401640
abort(this);
16411641
return;
16421642
}
1643+
const onError = options.onError;
1644+
16431645
if (err) {
1644-
process.nextTick(() => this.emit('error', err));
1646+
if (onError) {
1647+
onError(err);
1648+
} else {
1649+
this.destroy(err);
1650+
}
16451651
return;
16461652
}
1653+
16471654
if (!stat.isFile()) {
16481655
err = new errors.Error('ERR_HTTP2_SEND_FILE');
1649-
process.nextTick(() => this.emit('error', err));
1656+
if (onError) {
1657+
onError(err);
1658+
} else {
1659+
this.destroy(err);
1660+
}
16501661
return;
16511662
}
16521663

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

16841695
function afterOpen(session, options, headers, getTrailers, err, fd) {
16851696
const state = this[kState];
1697+
const onError = options.onError;
16861698
if (this.destroyed || session.destroyed) {
16871699
abort(this);
16881700
return;
16891701
}
16901702
if (err) {
1691-
process.nextTick(() => this.emit('error', err));
1703+
if (onError) {
1704+
onError(err);
1705+
} else {
1706+
this.destroy(err);
1707+
}
16921708
return;
16931709
}
16941710
state.fd = fd;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const http2 = require('http2');
8+
const assert = require('assert');
9+
const path = require('path');
10+
11+
const {
12+
HTTP2_HEADER_CONTENT_TYPE
13+
} = http2.constants;
14+
15+
const server = http2.createServer();
16+
server.on('stream', (stream) => {
17+
const file = path.join(process.cwd(), 'not-a-file');
18+
stream.respondWithFile(file, {
19+
[HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
20+
}, {
21+
onError(err) {
22+
common.expectsError({
23+
code: 'ENOENT',
24+
type: Error,
25+
message: `ENOENT: no such file or directory, open '${file}'`
26+
})(err);
27+
28+
stream.respond({ ':status': 404 });
29+
stream.end();
30+
},
31+
statCheck: common.mustNotCall()
32+
});
33+
});
34+
server.listen(0, () => {
35+
36+
const client = http2.connect(`http://localhost:${server.address().port}`);
37+
const req = client.request();
38+
39+
req.on('response', common.mustCall((headers) => {
40+
assert.strictEqual(headers[':status'], 404);
41+
}));
42+
req.on('data', common.mustNotCall());
43+
req.on('end', common.mustCall(() => {
44+
client.destroy();
45+
server.close();
46+
}));
47+
req.end();
48+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const http2 = require('http2');
8+
const assert = require('assert');
9+
10+
const {
11+
HTTP2_HEADER_CONTENT_TYPE
12+
} = http2.constants;
13+
14+
const server = http2.createServer();
15+
server.on('stream', (stream) => {
16+
stream.respondWithFile(process.cwd(), {
17+
[HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
18+
}, {
19+
onError(err) {
20+
common.expectsError({
21+
code: 'ERR_HTTP2_SEND_FILE',
22+
type: Error,
23+
message: 'Only regular files can be sent'
24+
})(err);
25+
26+
stream.respond({ ':status': 404 });
27+
stream.end();
28+
},
29+
statCheck: common.mustNotCall()
30+
});
31+
});
32+
server.listen(0, () => {
33+
34+
const client = http2.connect(`http://localhost:${server.address().port}`);
35+
const req = client.request();
36+
37+
req.on('response', common.mustCall((headers) => {
38+
assert.strictEqual(headers[':status'], 404);
39+
}));
40+
req.on('data', common.mustNotCall());
41+
req.on('end', common.mustCall(() => {
42+
client.destroy();
43+
server.close();
44+
}));
45+
req.end();
46+
});

0 commit comments

Comments
 (0)