Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parallel/test-http2-misbehaving-multiplex is flaky #54859

Closed
lpinca opened this issue Sep 9, 2024 · 1 comment · Fixed by #54872
Closed

parallel/test-http2-misbehaving-multiplex is flaky #54859

lpinca opened this issue Sep 9, 2024 · 1 comment · Fixed by #54872
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. linux Issues and PRs related to the Linux platform. macos Issues and PRs related to the macOS platform / OSX.

Comments

@lpinca
Copy link
Member

lpinca commented Sep 9, 2024

Test

test-http2-misbehaving-multiplex

Platform

Linux x64, macOS x64

Console output

=== release test-http2-misbehaving-multiplex ===
Path: parallel/test-http2-misbehaving-multiplex
node:assert:126
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected "actual" to be strictly unequal to: 5
    at Http2Server.<anonymous> (/home/luigi/node/test/parallel/test-http2-misbehaving-multiplex.js:45:10)
    at Http2Server.<anonymous> (/home/luigi/node/test/common/index.js:488:15)
    at Http2Server.emit (node:events:520:28)
    at ServerHttp2Session.sessionOnStream (node:internal/http2/core:3011:19)
    at ServerHttp2Session.emit (node:events:520:28)
    at emit (node:internal/http2/core:329:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:93:22) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 5,
  expected: 5,
  operator: 'notStrictEqual'
}

Node.js v23.0.0-pre
Command: out/Release/node --expose-internals /home/luigi/node/test/parallel/test-http2-misbehaving-multiplex.js


[01:25|% 100|+ 9999|-   1]: Done

Failed tests:
out/Release/node --expose-internals /home/luigi/node/test/parallel/test-http2-misbehaving-multiplex.js

Build links

Additional information

No response

@lpinca lpinca added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Sep 9, 2024
@github-actions github-actions bot added linux Issues and PRs related to the Linux platform. macos Issues and PRs related to the macOS platform / OSX. labels Sep 9, 2024
@lpinca
Copy link
Member Author

lpinca commented Sep 10, 2024

The problem is that when client.write(id1.data) is called for the second time

client.write(id1.data, () => {

the corresponding http2 stream might be fully closed and it seems that in that case no error is raised. Here is a test case that always fails (no error is raised).

'use strict';

const h2 = require('http2');
const net = require('net');
const h2test = require('../common/http2');

const server = h2.createServer();
server.on('stream', function (stream) {
  stream.respond();
  stream.end('ok');
});

server.on('session', function (session) {
  session.on('error', console.error);
});

const settings = new h2test.SettingsFrame();
const settingsAck = new h2test.SettingsFrame(true);
// HeadersFrame(id, payload, padding, END_STREAM)
const id1 = new h2test.HeadersFrame(1, h2test.kFakeRequestHeaders, 0, true);

server.listen(0, () => {
  const client = net.connect(server.address().port, function () {
    client.write(h2test.kClientMagic);
    client.write(settings.data);
    client.write(settingsAck.data);
    client.write(id1.data);
    setTimeout(function () {
      client.write(id1.data)
    }, 100);
  });

  client.on('close', function () {
    server.close();
  });

  client.resume();
});

This seems to be by design. The following callbacks

node/src/node_http2.cc

Lines 517 to 518 in 741004a

nghttp2_session_callbacks_set_on_invalid_frame_recv_callback(
callbacks_, OnInvalidFrame);

nghttp2_session_callbacks_set_error_callback2(callbacks_, OnNghttpError);

are not called. It just fails silently.

lpinca added a commit to lpinca/node that referenced this issue Sep 10, 2024
lpinca added a commit to lpinca/node that referenced this issue Sep 12, 2024
lpinca added a commit to lpinca/node that referenced this issue Sep 12, 2024
nodejs-github-bot pushed a commit that referenced this issue Sep 12, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
RafaelGSS pushed a commit that referenced this issue Sep 16, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
RafaelGSS pushed a commit that referenced this issue Sep 17, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this issue Sep 22, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this issue Sep 26, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this issue Oct 2, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this issue Oct 2, 2024
Fixes: #54859
PR-URL: #54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
Fixes: nodejs#54859
PR-URL: nodejs#54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
Fixes: nodejs#54859
PR-URL: nodejs#54872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. linux Issues and PRs related to the Linux platform. macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant