feat: replace grpc transport with fibp (fila binary protocol)#5
Merged
vieiralucas merged 5 commits intomainfrom Mar 26, 2026
Merged
feat: replace grpc transport with fibp (fila binary protocol)#5vieiralucas merged 5 commits intomainfrom
vieiralucas merged 5 commits intomainfrom
Conversation
rewrites the transport layer from gRPC/protobuf to FIBP: a custom length-prefixed binary protocol over raw TCP (or TLS). removes the grpc and google-protobuf gem dependencies entirely. new files: - lib/fila/transport.rb — tcp connection, handshake, frame read/write, corr-id multiplexing, reader thread, tls/mtls, auth frame - lib/fila/codec.rb — fibp binary encoding/decoding for enqueue, consume, ack, nack (pack/unpack, no external deps) updated: - lib/fila/client.rb — uses transport + codec instead of grpc stub - lib/fila/batcher.rb — uses transport directly instead of grpc stub - lib/fila/errors.rb — rpcerror now carries a fibp error code - lib/fila/version.rb — bumped to 0.5.0 - fila-client.gemspec — removed grpc/google-protobuf deps - test/test_helper.rb — admin ops (create_queue, wait_for_ready) reimplemented over fibp; removed grpc admin stub - test/test_tls_auth.rb — error code assertions updated to fibp codes - test/test_batch.rb — removed gRPC-specific lazy-connect assumption - README.md — documents fibp transport, removes gRPC references deleted: lib/fila/proto/ (generated protobuf ruby files) deleted: proto/ (proto source files)
- remove redundant require 'thread' (stdlib auto-loaded) - fix extra spacing on constants - use class keyword instead of Class.new for ConnectionClosed - rename short param names (op → opcode, n → num_bytes, op → operation) - use rescue as block rescue (not modifier form) in drain_pending and batcher - use anybits? for bitflag check (Style/BitwisePredicate) - remove redundant begin blocks - remove unnecessary rubocop:disable directives - extract read_str16/read_headers helpers to reduce decode_consume_push complexity - fix $1/$2 perl backrefs to named match captures - rewrite parse_addr to avoid duplicate branch body - fix gemspec description string literals and line length
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 20 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/fila/codec.rb">
<violation number="1" location="lib/fila/codec.rb:42">
P1: `decode_consume_push` skips the documented leading `msg_count:u16`, which can misalign all subsequent field decoding for consume push frames.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:94">
P3: The docs incorrectly say the API key is sent on every request; in this implementation it is sent once via an AUTH frame during connection setup.</violation>
</file>
<file name="lib/fila/client.rb">
<violation number="1" location="lib/fila/client.rb:221">
P1: `enqueue_single` raises `QueueNotFoundError` for every per-message failure, including non-queue-not-found errors. The old code had a `case` on the error code to distinguish queue-not-found from other failures. The per-message `err_code` is present in the FIBP response but is discarded in `Codec.decode_enqueue_response` (`_err_code`), so the client can't differentiate. The error code should be preserved in `EnqueueResult` and checked here.</violation>
</file>
<file name="lib/fila/transport.rb">
<violation number="1" location="lib/fila/transport.rb:69">
P0: **Deadlock**: `send_auth` is called before `start_reader`, but `send_auth` calls `request()` which blocks waiting for a response that only the reader thread can deliver. This deadlocks every connection that uses an API key.
Swap the two lines so the reader thread is running before the AUTH request is sent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
the fibp handshake-based readiness check fails against the existing gRPC server binary (dev-latest release) since it speaks HTTP/2, not FIBP. switch to a plain tcp connect which reliably detects when the port is accepting connections, regardless of the server protocol. the subsequent fibp operations will produce explicit errors if the server does not speak FIBP.
p0 (transport.rb): deadlock — start_reader before send_auth so the reader thread is running to deliver the auth response. previously send_auth was called before start_reader, blocking forever on a response that no thread would deliver. p1 (codec.rb): decode_consume_push was missing the leading msg_count:u16be field. added read of _msg_count at position 0 to align all subsequent field offsets correctly. p1 (client.rb + batcher.rb): enqueue_single and result_to_outcome raised queuenotfounderror for all per-message failures. preserved error_code in enqueueresult and added raise_enqueue_error helper that distinguishes queue_not_found (error_code 1) from other failures (raises rpcerror with the actual code). p3 (readme.md): corrected api key auth docs to say the key is sent once as an auth frame at connection setup, not on every request.
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/fila/codec.rb">
<violation number="1" location="lib/fila/codec.rb:82">
P1: `decode_consume_push` ignores `msg_count`, which can drop messages for multi-message frames and fail on empty frames.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # @return [ConsumeMessage, nil] the first message in the frame | ||
| def decode_consume_push(payload) | ||
| pos = 0 | ||
| _msg_count, pos = read_u16(payload, pos) |
There was a problem hiding this comment.
P1: decode_consume_push ignores msg_count, which can drop messages for multi-message frames and fail on empty frames.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/fila/codec.rb, line 82:
<comment>`decode_consume_push` ignores `msg_count`, which can drop messages for multi-message frames and fail on empty frames.</comment>
<file context>
@@ -70,10 +70,16 @@ def encode_consume(queue, initial_credits: 256)
+ # @return [ConsumeMessage, nil] the first message in the frame
def decode_consume_push(payload)
pos = 0
+ _msg_count, pos = read_u16(payload, pos)
msg_id, pos = read_str16(payload, pos)
fairness_key, pos = read_str16(payload, pos)
</file context>
- use SystemCallError instead of specific errno subclasses to handle all network errors during server readiness polling (e.g. ETIMEDOUT) - enqueueresult now carries error_code for per-message error differentiation (already committed in previous fix commit)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
New files:
Updated:
Deleted:
CI status
Addressed Cubic findings:
Test plan
🤖 Generated with Claude Code