Skip to content

Commit

Permalink
workaround for github.com/nodejs/node/issues/37849 (#2655)
Browse files Browse the repository at this point in the history
* workaround for github.com/nodejs/node/issues/37849

* add test
  • Loading branch information
LavrovArtem authored Jun 21, 2021
1 parent f95a1f2 commit d70621e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 4 deletions.
13 changes: 12 additions & 1 deletion src/request-pipeline/destination-request/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,24 @@ export default class DestinationRequest extends EventEmitter implements Destinat
this._send();
}

private static _isHttp2ProtocolError (err: Error) {
return err['code'] === 'ERR_HTTP2_STREAM_ERROR' && err.message.includes('NGHTTP2_PROTOCOL_ERROR');
}

private _sendRealThroughHttp2 (session: ClientHttp2Session) {
const reqHeaders = formatRequestHttp2Headers(this.opts);
const endStream = !this.opts.body.length;
const stream = session.request(reqHeaders, { endStream });

stream.setTimeout(this.timeout, () => this._onTimeout());
stream.on('error', (err: Error) => this._onError(err));
stream.on('error', (err: Error) => {
if (DestinationRequest._isHttp2ProtocolError(err)) {
session.destroy();
this._sendReal();
}
else
this._onError(err);
});
stream.on('response', headers => {
const http2res = createResponseLike(stream, headers);

Expand Down
54 changes: 51 additions & 3 deletions test/server/proxy/http2-client-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ const { HTTP2_HEADER_STATUS } = require('http2').constants;
const { expect } = require('chai');
const request = require('request-promise-native');
const logger = require('../../../lib/utils/logger');
const { clearSessionsCache } = require('../../../lib/request-pipeline/destination-request/http2');
const { noop } = require('lodash');
const http2Utils = require('../../../lib/request-pipeline/destination-request/http2');
const { CROSS_DOMAIN_SERVER_PORT } = require('../common/constants');

const {
Expand Down Expand Up @@ -39,7 +40,6 @@ describe('https proxy', () => {
}

before(() => {

const crossDomainDestinationServer = createDestinationServer(CROSS_DOMAIN_SERVER_PORT, true);
const { app: httpsApp } = crossDomainDestinationServer;

Expand Down Expand Up @@ -73,7 +73,7 @@ describe('https proxy', () => {
restoreLoggerFn(logger.destination, 'onHttp2SessionCreated');
restoreLoggerFn(logger.destination, 'onHttp2Unsupported');
http2Server.close();
clearSessionsCache();
http2Utils.clearSessionsCache();
httpsServer.close();
});

Expand Down Expand Up @@ -124,4 +124,52 @@ describe('https proxy', () => {
expect(logs[1][0]).eql('https://127.0.0.1:2002');
});
});

// https://github.com/nodejs/node/issues/37849
it('Should resend a request through https if http2 stream is emitted a protocol error', () => {
const sessionMock = {
destroyCalled: false,

request: () => ({
setTimeout: noop,
write: noop,

end: function () {
setImmediate(() => this._errorHandler({
code: 'ERR_HTTP2_STREAM_ERROR',
message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR'
}));
},

on: function (eventName, fn) {
if (eventName === 'error')
this._errorHandler = fn;
}
}),

destroy: function () {
this.destroyCalled = true;
}
};

session.id = 'sessionId';

const storedGetHttp2Session = http2Utils.getHttp2Session;
const proxyUrl = getProxyUrl('https://127.0.0.1:2002/stylesheet');

http2Utils.getHttp2Session = () => sessionMock;

proxy.openSession('https://127.0.0.1:2000/', session);

return request(proxyUrl, { form: { key: 'value' } })
.then(body => {
http2Utils.getHttp2Session = storedGetHttp2Session;

const expected = fs.readFileSync('test/server/data/stylesheet/expected.css').toString();

compareCode(body, expected);
expect(logs[0]).eql('onHttp2Stream');
expect(sessionMock.destroyCalled).to.be.true;
});
});
});

0 comments on commit d70621e

Please sign in to comment.