Node: TypeError occur with http2ServerRequest.socket.destroy()

[sogaani]

I met following error when call http2ServerRequest.socket.destroy();

internal/http2/core.js:1467
  const ret = stream[kHandle].trailers(headersList);
                              ^

TypeError: Cannot read property 'trailers' of undefined
    at Immediate.finishSendTrailers (internal/http2/core.js:1467:31)
    at runCallback (timers.js:695:11)
    at tryOnImmediate (timers.js:665:5)
    at processImmediate (timers.js:647:5)

The minimal code to reproduce.

const http2 = require('http2');
const {
    HTTP2_HEADER_PATH,
    HTTP2_HEADER_METHOD,
} = http2.constants;

const app = http2.createServer((req, res) => {
    res.end('hello');
    setImmediate( ()=>{ req.socket.destroy(); });
});

app.listen(8080, () => {
    const session = http2.connect('http://localhost:8080');
    const request = session.request({ [HTTP2_HEADER_PATH]: '/', [HTTP2_HEADER_METHOD]: 'get' });
    request.once('response', (headers, flags) => {
        let data = '';
        request.on('data', (chunk) => { data += chunk; });
        request.on('end', () => { console.log(data); });
    });
    request.end();
});

node: version

$ node -v
v10.10.0

I guess we should check if stream destroyed in finishSendTrailers, or we should not use setImmeidiate to call finishSendTrailers.
Currently we can call http2Session.destroy(); which delete some property while finishSendTrailers is listed in nodejs eventloop and it lead to an error.

[lovinglyy]

Can reproduce in the same version. I also tested in a experimental http2 version, node v8.9.4 and I couldn't reproduce, checking if it's destroyed looks concise in my view, we could also directly check if the stream is defined, as the issue is mainly reproducible by calling socket.destroy() with setImmediate.

[apapirovski]

Is there a specific reason you need to call setImmediate( ()=>{ req.socket.destroy(); });? Like that's not a clean way to close the connection...

[sogaani]

I found this issue with Express test case. Express call request.socket.destroy() in finalize function when error occur in request handler or middleware and already send header and body frame.
I'm not sure what is clean way to close connection, but I don't think it is strange close connection when server internal error occurs and already send header and body frame.

Example.

var app = express();

app.get('/', function(req, res){
  res.end('yay');
  throw new Error('boom');
});

[mcollina]

I'm working on a fix.

[mcollina]

Fixed in https://github.com/nodejs/node/pull/22896.