fix: integration test readiness for fibp

Merged
VI
8mo ago
+305-92

Summary

This PR fixes multiple bugs that caused integration tests to fail with "fila-server failed to start within 10s" and other errors. The root cause was a combination of an insufficient readiness probe and several protocol encoding bugs.

Readiness probe (test/test_helper.rb)

  • Was: plain TCP connect — succeeds as soon as the listener binds, before FIBP is ready
  • Now: full FIBP handshake probe (FIBP\x01\x00 → echo) so we only proceed when the server can actually serve FIBP requests
  • mTLS: probe now supplies client cert/key when the server requires mTLS, so the TLS handshake succeeds during the probe
  • Port race: retry server spawn up to 3 times when another process races to bind the chosen port

Admin create_queue encoding (test/test_helper.rb)

  • Admin ops use protobuf-encoded payloads. The old code sent a custom binary format; the server rejected it with "failed to decode Protobuf message: invalid tag value: 0"
  • Fixed: hand-encode CreateQueueRequest { name: "..." } in proto3 wire format (field 1, wire type 2)

Consume push routing (lib/fila/transport.rb)

  • Server-push frames carry corr_id=0 regardless of the consume request's corr_id. The push queue was registered only at the request corr_id, so push frames were silently dropped
  • Fixed: register push queue at corr_id=0; use a separate one-shot queue for the consume ACK

OP_AUTH payload format (lib/fila/transport.rb)

  • Auth frame payload is raw key bytes — no length prefix. The old code prepended a u16 length, which the server decoded as the first two bytes of the key, causing "invalid or missing api key"

OP_ERROR frame parsing (lib/fila/transport.rb)

  • OP_ERROR frames carry plain UTF-8 text, not code:u16 + len:u16 + msg. The old parser read garbage bytes as an error code
  • Fixed: parse text directly; derive error type (RPCError, QueueNotFoundError, etc.) from message content

Codec module function visibility (lib/fila/codec.rb)

  • module_function only applies to methods defined before private. Helper methods (encode_message, encode_str16, read_*) were after private, making them unavailable as Codec.method_name from the batcher thread
  • Fixed: move all helpers before private; remove the private block entirely
  • alias decode_nack_response replaced with an explicit def (aliases don't inherit module_function)

Consume push wire format (lib/fila/codec.rb)

  • Decoder had a phantom queue_id: string16 field that doesn't exist in the server's wire format, causing nil.unpack1 on every consumed message
  • Fixed: remove the field; pass queue name from the caller (Ruby SDK stores it from the consume request, same as the Rust SDK)

Batcher error propagation (lib/fila/batcher.rb)

  • Re-wrapping exceptions in Fila::Error.new(e.message) lost the specific type (QueueNotFoundError, RPCError). Tests asserting assert_raises(Fila::RPCError) would fail
  • Fixed: broadcast the original exception directly

Test plan

  • All 28 integration tests pass locally with the dev binary
  • test/test_client.rb — 3/3 passing
  • test/test_batch.rb — 16/16 passing
  • test/test_tls_auth.rb — 9/9 passing (TLS, mTLS, API key, backward compat)

🤖 Generated with Claude Code

Summary by cubic

Fixes flaky integration tests by using a real FIBP readiness probe, correcting protocol mismatches, and guarding TLS tests when the server lacks TLS. Startup is reliable now, server push/auth/error paths match the wire format, and corr_id wraparound no longer collides with server-push.

  • Bug Fixes
    • Readiness: perform a full FIBP handshake (with mTLS when needed); retry port bind up to 3 times.
    • Admin: send protobuf-encoded CreateQueueRequest for create_queue.
    • Consume push: register push queue at corr_id=0 and use a one-shot ACK queue; client passes queue name to the decoder.
    • Correlation IDs: reserve 0 for server-push; cycle request IDs through 1..0xFFFFFFFF to avoid collisions.
    • Auth: OP_AUTH payload is raw API key bytes (no length prefix).
    • Errors: parse OP_ERROR as UTF-8 text and map to typed errors by message.
    • Codec: expose helpers via module_function, define decode_nack_response, remove the non-existent queue_id from consume push decoding.
    • Batcher: re-broadcast the original exception type (preserves QueueNotFoundError, RPCError, etc.).
    • TLS tests: probe TLS support at load time and skip when unsupported to avoid server panics.