stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 672 forks source link

Fix/http stall on invalid message #5491

Closed jcnelson closed 14 hours ago

jcnelson commented 17 hours ago

This fixes #5490, and fixes the problem we were seeing in #5296 and #4997. The fundamental problem was that the HTTP session state machine was not correctly treating server-generated HTTP error responses as having completed once they were flushed. This PR fixes this, and removes some now-needless (and buggy) code that had earlier attempted (and failed) to address this problem.

Let's get this into develop so we can turn around and fix #5296 and #4997.

wileyj commented 15 hours ago

hmm, this test is still failing here (and pasing elsewhere): tests::nakamoto_integrations::follower_bootup_across_multiple_cycles

INFO [1732214479.166592] [stackslib/src/net/rpc.rs:551] [p2p-(127.0.0.1:62639,127.0.0.1:50473)] Handled StacksHTTPRequest, verb: GET, path: /v2/info, processing_time_ms: 0, latency_ms: 0, conn_id: 291, peer_addr: 127.0.0.1:56920, p2p_msg: None
thread 'tests::nakamoto_integrations::follower_bootup_across_multiple_cycles' panicked at testnet/stacks-node/src/tests/nakamoto_integrations.rs:3891:6:
INFO [1732214479.167226] [testnet/stacks-node/src/tests/nakamoto_integrations.rs:3882] [tests::nakamoto_integrations::follower_bootup_across_multiple_cycles] Follower tip is now cf481cf4f2faf436a1ca371400ade13f5aa731b6/575cd2da0bbfd4667d09584b3c8248fcdc475d1a36c2ef203402ed10f550465f
called `Result::unwrap()` on an `Err` value: "Timed out"
ERRO [1732214479.167241] [testnet/stacks-node/src/tests/nakamoto_integrations.rs:652] [tests::nakamoto_integrations::follower_bootup_across_multiple_cycles] Timed out waiting for check to process
stack backtrace:
   0:     0x55889fad884a - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::h304520fd6a30aa07
   1:     0x55889fb07b1b - core::fmt::write::hf5713710ce10ff22
   2:     0x55889fad35d3 - std::io::Write::write_fmt::hda708db57927dacf
   3:     0x55889fad9f22 - std::panicking::default_hook::{{closure}}::he1ad87607d0c11c5
   4:     0x55889fad9b8e - std::panicking::default_hook::h81c8cd2e7c59ee33
   5:     0x55889fada7af - std::panicking::rust_panic_with_hook::had2118629c312a4a
   6:     0x55889fada497 - std::panicking::begin_panic_handler::{{closure}}::h7fa5985d111bafa2
   7:     0x55889fad8d29 - std::sys::backtrace::__rust_end_short_backtrace::h704d151dbefa09c5
   8:     0x55889fada124 - rust_begin_unwind
   9:     0x55889d78bd23 - __covrec_42075DFFF1D7CA83
  10:     0x55889d78c2d6 - __covrec_4E37FFB6DC978C27
  11:     0x55889de605e5 - stacks_node[51ab313562974921]::tests::nakamoto_integrations::follower_bootup_across_multiple_cycles
  12:     0x55889ddeae69 - <stacks_node[51ab313562974921]::tests::nakamoto_integrations::follower_bootup_across_multiple_cycles::{closure#0} as core[d89802b8f5f07590]::ops::function::FnOnce<()>>::call_once
  13:     0x55889e1f788b - test::__rust_begin_short_backtrace::hbd02dbd41797127f
  14:     0x55889e1f7178 - test::run_test::{{closure}}::h65f9c94bff1a7861
  15:     0x55889e1bc544 - std::sys::backtrace::__rust_begin_short_backtrace::h160ed2f20abbd124
  16:     0x55889e1bfbf2 - core::ops::function::FnOnce::call_once{{vtable.shim}}::h262984108de629b4
  17:     0x55889fae126b - std::sys::pal::unix::thread::Thread::new::thread_start::hcdbd1049068002f4
  18:     0x7fd2fa494ac3 - <unknown>
  19:     0x7fd2fa526850 - <unknown>
  20:                0x0 - <unknown>
test tests::nakamoto_integrations::follower_bootup_across_multiple_cycles ... FAILED
jcnelson commented 15 hours ago

@wileyj I've fixed that in a different branch