the Stream is closed if app pushes unused assets

[budarin]

• Version: 10.1.0
• Platform: Windows Server 2008R2 64
• Subsystem: any browser any OS

If push some assets which will not be used with browser - stream closes on the second request
Page requires only <script src="script.js"></script>, but if to push something that is not using on the page - the next page refresh will close the http2 stream

        const pushAsset = (stream, file) => {
            const filePath = path.resolve(file.filePath);
        
            stream.pushStream({ [HTTP2_HEADER_PATH]: file.path }, (err, pushStream) => {
                pushStream.respondWithFile(filePath, file.headers, {
                    onError: (err) => {
                        respondToStreamError(err, stream);
                    }
                });
            });
        };

        const cssFile = {
            path: '/style.css',
            filePath: './style.css',
            headers: {
                'content-type': 'text/css'
            }
        };
        const cssFile1 = {
            path: '/style1.css',
            filePath: './style1.css',
            headers: {
                'content-type': 'text/css'
            }
        };
        const jsFile = {
            path: '/script.js',
            filePath: './script.js',
            headers: {
                'content-type': 'application/javascript'
            }
        };

        pushAsset(stream, jsFile);
        pushAsset(stream, cssFile);   //<-- unused
        pushAsset(stream, cssFile1);   //<-- unused

The error:

events.js:167
      throw er; // Unhandled 'error' event
      ^

Error [ERR_HTTP2_STREAM_ERROR]: Stream closed with error code NGHTTP2_REFUSED_STREAM
    at ServerHttp2Stream._destroy (internal/http2/core.js:1871:13)
    at ServerHttp2Stream.destroy (internal/streams/destroy.js:32:8)
    at ServerHttp2Stream.[maybe-destroy] (internal/http2/core.js:1887:12)
    at Http2Stream.onStreamClose [as onstreamclose] (internal/http2/core.js:346:26)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)

Declaration of extra assets should not crush the tream

[budarin]

Here is the demo project

try to uncomment these lines:

        // pushAsset(stream, jsFile1);
        // pushAsset(stream, jsFile2);

and make some quick page refreshes - it makes the error.

[ryzokuken]

/cc @nodejs/http2

[apapirovski]

This seems as expected to me. Add an error handler that ignores NGHTTP2_REFUSED_STREAM errors. See the spec: https://http2.github.io/http2-spec/#rfc.section.7

[budarin]

@apapirovski
I'm sorry, could you provide the code, please.

I don't quite understand what you mean.

[apapirovski]

Sorry, should've been clearer, you can use something like this to make sure those errors don't take down your app:

pushStream.on('error', (err) => {
  const isRefusedStream = err.code === 'ERR_HTTP2_STREAM_ERROR' &&
                          pushStream.rstCode === NGHTTP2_REFUSED_STREAM;
  if (!isRefusedStream)
    throw err;
});

[budarin]

It didnt helped - after some refreshes here is another error:

npm[10144]: src\node_file.cc:215: Assertion `!reading_' failed.
 1: node::DecodeWrite
 2: node::DecodeWrite
 3: uv_loop_fork
 4: uv_loop_fork
 5: v8::internal::interpreter::BytecodeDecoder::Decode
 6: v8::internal::RegExpImpl::Exec
 7: v8::internal::RegExpImpl::Exec
 8: v8::internal::RegExpImpl::Exec
 9: 000000B116304281

Please review the code:

const pushAsset = (stream, file) => {
    const filePath = path.resolve(path.join(serverRoot, file.filePath));

    stream.pushStream({ [HTTP2_HEADER_PATH]: file.path }, { parent: stream.id }, (err, pushStream) => {
        if (!err) {
            console.log(">> Pushing:", file.path);

            pushStream.on('error', (err) => {
                console.log('pushStream.on error', err);

                const isRefusedStream = err.code === 'ERR_HTTP2_STREAM_ERROR' &&
                    pushStream.rstCode === NGHTTP2_REFUSED_STREAM;

                if (isRefusedStream)  {
                    return;
                }

                respondToStreamError(err, pushStream);
            });

            pushStream.respondWithFile(filePath, file.headers);
        }

        if (err) {
            console.log(">> Pushing error:", err);
        }
    });
};

[budarin]

And even not pushing unnessesary assets, but quickly refresh the page - you will get the error:

npm[5912]: src\node_http2.cc:1934: Assertion `!this->IsDestroyed()' failed.
 1: node::DecodeWrite
 2: node::DecodeWrite
 3: ENGINE_get_ctrl_function
 4: DH_get0_engine
 5: std::basic_ostream<char,std::char_traits<char> >::basic_ostream<char,std::char_traits<char> >
 6: uv_loop_fork
 7: uv_fs_get_statbuf
 8: uv_dlerror
 9: node::CreatePlatform
10: node::CreatePlatform
11: node::Start
12: v8::SnapshotCreator::`default constructor closure'
13: v8::internal::AsmJsScanner::IsNumberStart
14: BaseThreadInitThunk
15: RtlUserThreadStart

[apapirovski]

Sounds like there are potentially issues in our C++ pipe code.

ping @addaleax

[ryzokuken]

@budarin I could take a look into this, but could you bring me up to speed by sharing the complete relevant code for the server script? Or is the stream object a simple newly initialized stream?

[budarin]

here is the repository for experiments and testing http2 project

[ryzokuken]

@budarin Umm, I uncommented the two lines and reloaded, the page loads up as expected. That said, I'm using chrome to connect. Hope that's not an issue?

[ryzokuken]

The two JavaScript files don't show up here, though? I think that's expected anyway.