runner: non-empty UpdateLogResponse for ack_index 0 (Connect-Go rejects empty)
52bf85b · gmorell · 2026-06-26 15:26
Files changed
modified
lib/git_gud_web/controllers/runner_controller.ex
+26
−15
@@ -592,21 +592,10 @@ defmodule GitGudWeb.RunnerController do
| 592 | 592 | defp respond(conn, %_{} = msg) do |
| 593 | 593 | case wire_format(conn) do |
| 594 | 594 | {:protobuf, echoed_ct} -> |
| 595 | − body = msg.__struct__.encode(msg) |> IO.iodata_to_binary() | |
| 596 | − | |
| 597 | − # Loud failure trumps a silent framing bug. Empty protobuf | |
| 598 | − # bodies look valid by spec but break Connect-Go's unary | |
| 599 | − # decoder. Each RPC handler is expected to populate at | |
| 600 | − # least one non-default field; this catches any new | |
| 601 | − # endpoint that forgets. | |
| 602 | − if byte_size(body) == 0 do | |
| 603 | − require Logger | |
| 604 | − | |
| 605 | − Logger.error( | |
| 606 | − "runner-protocol: empty protobuf body for #{inspect(msg.__struct__)}; " <> | |
| 607 | − "Connect-Go will reject. Populate at least one non-default field." | |
| 608 | − ) | |
| 609 | − end | |
| 595 | + body = | |
| 596 | + msg.__struct__.encode(msg) | |
| 597 | + |> IO.iodata_to_binary() | |
| 598 | + |> ensure_nonempty_body(msg) | |
| 610 | 599 | |
| 611 | 600 | conn |
| 612 | 601 | |> put_resp_content_type(echoed_ct, nil) |
@@ -623,6 +612,28 @@ defmodule GitGudWeb.RunnerController do
| 623 | 612 | end |
| 624 | 613 | end |
| 625 | 614 | |
| 615 | + # Connect-Go rejects an empty unary response body. The common case is | |
| 616 | + # UpdateLogResponse{ack_index: 0} — the runner's first/empty log batch | |
| 617 | + # — which proto3 serializes to nothing, stalling the runner's log | |
| 618 | + # handshake and ultimately failing an otherwise-successful job. Encode | |
| 619 | + # field 1 (ack_index) explicitly as varint 0 so the body is non-empty | |
| 620 | + # but still decodes to 0. Any other empty message is a forgotten-field | |
| 621 | + # bug — log loudly and leave it empty. | |
| 622 | + defp ensure_nonempty_body(<<>>, %UpdateLogResponse{}), do: <<8, 0>> | |
| 623 | + | |
| 624 | + defp ensure_nonempty_body(<<>>, %_{} = msg) do | |
| 625 | + require Logger | |
| 626 | + | |
| 627 | + Logger.error( | |
| 628 | + "runner-protocol: empty protobuf body for #{inspect(msg.__struct__)}; " <> | |
| 629 | + "Connect-Go will reject. Populate at least one non-default field." | |
| 630 | + ) | |
| 631 | + | |
| 632 | + <<>> | |
| 633 | + end | |
| 634 | + | |
| 635 | + defp ensure_nonempty_body(body, _msg), do: body | |
| 636 | + | |
| 626 | 637 | # Disable HTTP/1.1 keep-alive for runner-protocol responses. |
| 627 | 638 | # |
| 628 | 639 | # Bandit + forgejo-runner's Connect-Go HTTP/1.1 client get into |
Parents: 1239e9a