test data
499c875 · gmorell · 2026-05-22 18:01
Files changed
modified
lib/git_gud_web/controllers/runner_controller.ex
+34
−3
@@ -49,7 +49,12 @@ defmodule GitGudWeb.RunnerController do
| 49 | 49 | |
| 50 | 50 | def ping(conn, _params) do |
| 51 | 51 | with {:ok, %PingRequest{data: data}} <- decode_request(conn, PingRequest) do |
| 52 | − respond(conn, %PingResponse{data: data}) | |
| 52 | + # Connect-Go's unary decoder rejects 0-byte responses (see | |
| 53 | + # FetchTaskResponse + UpdateLogResponse for the same class of | |
| 54 | + # framing fix). Empty PingRequest.data → empty PingResponse, | |
| 55 | + # so default to "pong" when the client sent nothing. | |
| 56 | + reply = if data == "", do: "pong", else: data | |
| 57 | + respond(conn, %PingResponse{data: reply}) | |
| 53 | 58 | end |
| 54 | 59 | end |
| 55 | 60 |
@@ -409,6 +414,16 @@ defmodule GitGudWeb.RunnerController do
| 409 | 414 | |
| 410 | 415 | step = job && Workflows.ensure_log_step(job) |
| 411 | 416 | |
| 417 | + # Connect-Go decodes empty unary bodies as "method not | |
| 418 | + # implemented" and surfaces it as "unimplemented: 404 Not | |
| 419 | + # Found" — which corrupts the HTTP/1.1 keepalive stream when | |
| 420 | + # the actual content-length disagrees with what was written. | |
| 421 | + # `ack_index = 0` would encode to 0 bytes (proto3 omits default | |
| 422 | + # ints), so we always advertise at least 1 acked row when the | |
| 423 | + # request itself was non-zero — matches Forgejo's behavior of | |
| 424 | + # echoing back the high-water mark. | |
| 425 | + ack_index = max(index + length(rows), 1) | |
| 426 | + | |
| 412 | 427 | cond do |
| 413 | 428 | is_nil(step) -> |
| 414 | 429 | require Logger |
@@ -417,11 +432,11 @@ defmodule GitGudWeb.RunnerController do
| 417 | 432 | "update_log: no running job for runner_id=#{inspect(runner.id)} task_id=#{inspect(tid)}; acking" |
| 418 | 433 | ) |
| 419 | 434 | |
| 420 | − respond(conn, %UpdateLogResponse{ack_index: index + length(rows)}) | |
| 435 | + respond(conn, %UpdateLogResponse{ack_index: ack_index}) | |
| 421 | 436 | |
| 422 | 437 | true -> |
| 423 | 438 | :ok = Workflows.append_log(step, index, rows) |
| 424 | − respond(conn, %UpdateLogResponse{ack_index: index + length(rows)}) | |
| 439 | + respond(conn, %UpdateLogResponse{ack_index: ack_index}) | |
| 425 | 440 | end |
| 426 | 441 | else |
| 427 | 442 | nil -> error(conn, :unauthenticated, "bad token") |
@@ -503,6 +518,22 @@ defmodule GitGudWeb.RunnerController do
| 503 | 518 | {:protobuf, echoed_ct} -> |
| 504 | 519 | body = msg.__struct__.encode(msg) |> IO.iodata_to_binary() |
| 505 | 520 | |
| 521 | + # Loud failure trumps a silent framing bug. Empty protobuf | |
| 522 | + # bodies look valid by spec but break Connect-Go's unary | |
| 523 | + # decoder AND, downstream, the HTTP/1.1 keepalive stream | |
| 524 | + # (next request reads mid-stream — see Bandit.HTTPError | |
| 525 | + # `Request line HTTP error`). Each RPC handler is expected | |
| 526 | + # to populate at least one non-default field; this catches | |
| 527 | + # any new endpoint that forgets. | |
| 528 | + if byte_size(body) == 0 do | |
| 529 | + require Logger | |
| 530 | + | |
| 531 | + Logger.error( | |
| 532 | + "runner-protocol: empty protobuf body for #{inspect(msg.__struct__)}; " <> | |
| 533 | + "Connect-Go will reject. Populate at least one non-default field." | |
| 534 | + ) | |
| 535 | + end | |
| 536 | + | |
| 506 | 537 | conn |
| 507 | 538 | |> put_resp_content_type(echoed_ct, nil) |
| 508 | 539 | |> send_resp(200, body) |
Parents: e2011b6