From 1e93f6fd7ec3439c3622fc35de5be10907d93c7f Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Fri, 27 Feb 2026 21:18:32 +0100 Subject: [PATCH 1/6] fix: move parse_body plug after start_telemetry_span to prevent ArithmeticError When parse_body halted a request before start_telemetry_span had executed, the custom halt/1 would call end_telemetry_span which tried to compute `System.monotonic_time() - nil`, causing an ArithmeticError. Fixes #3919 Co-Authored-By: Claude Opus 4.6 --- packages/sync-service/lib/electric/plug/serve_shape_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sync-service/lib/electric/plug/serve_shape_plug.ex b/packages/sync-service/lib/electric/plug/serve_shape_plug.ex index 91ee5bd5c3..03631419f6 100644 --- a/packages/sync-service/lib/electric/plug/serve_shape_plug.ex +++ b/packages/sync-service/lib/electric/plug/serve_shape_plug.ex @@ -13,10 +13,10 @@ defmodule Electric.Plug.ServeShapePlug do require Logger plug :fetch_query_params - plug :parse_body # start_telemetry_span needs to always be the first plug after fetching query params. plug :start_telemetry_span + plug :parse_body plug :put_resp_content_type, "application/json" plug :validate_request From d28d0f02a0cc2bdaf60c6d42055737d28c3768fa Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Fri, 27 Feb 2026 21:18:57 +0100 Subject: [PATCH 2/6] chore: add changeset for telemetry ArithmeticError fix Co-Authored-By: Claude Opus 4.6 --- .changeset/fix-telemetry-arithmetic-error.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-telemetry-arithmetic-error.md diff --git a/.changeset/fix-telemetry-arithmetic-error.md b/.changeset/fix-telemetry-arithmetic-error.md new file mode 100644 index 0000000000..f14a272029 --- /dev/null +++ b/.changeset/fix-telemetry-arithmetic-error.md @@ -0,0 +1,5 @@ +--- +"@core/sync-service": patch +--- + +Fixed ArithmeticError in `ServeShapePlug.end_telemetry_span/2` when `parse_body` halts before the telemetry span is started. Moved `parse_body` plug after `start_telemetry_span` in the pipeline. From bb8f5258700b35fccfe1f7910ecf98d9f2962bae Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Fri, 27 Feb 2026 22:03:49 +0100 Subject: [PATCH 3/6] test: add tests for parse_body error paths in ServeShapePlug These tests verify that POST requests with invalid JSON or non-object JSON bodies return proper 400 responses without crashing. Before the plug ordering fix, these paths would trigger an ArithmeticError in end_telemetry_span. Co-Authored-By: Claude Opus 4.6 --- .../electric/plug/serve_shape_plug_test.exs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs b/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs index 6241f1f264..5de23c5257 100644 --- a/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs +++ b/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs @@ -1028,6 +1028,42 @@ defmodule Electric.Plug.ServeShapePlugTest do end end + describe "parse_body error handling" do + setup :with_lsn_tracker + + setup ctx do + {:via, _, {registry_name, registry_key}} = + Electric.Shapes.Supervisor.name(ctx.stack_id) + + {:ok, _} = Registry.register(registry_name, registry_key, nil) + set_status_to_active(ctx) + + patch_storage(for_shape: fn @test_shape_handle, _opts -> @test_opts end) + + :ok + end + + test "returns 400 for invalid JSON body without crashing", ctx do + conn = + Plug.Test.conn(:post, "/?offset=-1", "not valid json") + |> put_req_header("content-type", "application/json") + |> call_serve_shape_plug(ctx) + + assert conn.status == 400 + assert %{"error" => "Invalid JSON in request body"} = Jason.decode!(conn.resp_body) + end + + test "returns 400 for non-object JSON body without crashing", ctx do + conn = + Plug.Test.conn(:post, "/?offset=-1", Jason.encode!(["an", "array"])) + |> put_req_header("content-type", "application/json") + |> call_serve_shape_plug(ctx) + + assert conn.status == 400 + assert %{"error" => "Request body must be a JSON object"} = Jason.decode!(conn.resp_body) + end + end + describe "stack not ready" do test "returns 503", ctx do conn = From 2795cc8afd39ba843ba108dcf7ac0c34c8b86a02 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Mon, 2 Mar 2026 11:52:28 +0100 Subject: [PATCH 4/6] fix: move put_resp_content_type to be first plug in pipeline Ensures all error responses (including those from parse_body) carry the correct Content-Type: application/json header. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> --- packages/sync-service/lib/electric/plug/serve_shape_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sync-service/lib/electric/plug/serve_shape_plug.ex b/packages/sync-service/lib/electric/plug/serve_shape_plug.ex index 03631419f6..062076e44d 100644 --- a/packages/sync-service/lib/electric/plug/serve_shape_plug.ex +++ b/packages/sync-service/lib/electric/plug/serve_shape_plug.ex @@ -12,12 +12,12 @@ defmodule Electric.Plug.ServeShapePlug do require Logger + plug :put_resp_content_type, "application/json" plug :fetch_query_params # start_telemetry_span needs to always be the first plug after fetching query params. plug :start_telemetry_span plug :parse_body - plug :put_resp_content_type, "application/json" plug :validate_request # Admission control is applied here, but note: From 96b3cee3cc297926e14c7d9ccb9d3eb62de1e568 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Mon, 2 Mar 2026 11:52:32 +0100 Subject: [PATCH 5/6] test: add tests for oversized body and body read failure in parse_body Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> --- .../electric/plug/serve_shape_plug_test.exs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs b/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs index 5de23c5257..8519dc8abe 100644 --- a/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs +++ b/packages/sync-service/test/electric/plug/serve_shape_plug_test.exs @@ -1062,6 +1062,30 @@ defmodule Electric.Plug.ServeShapePlugTest do assert conn.status == 400 assert %{"error" => "Request body must be a JSON object"} = Jason.decode!(conn.resp_body) end + + test "returns 413 for oversized body without crashing", ctx do + Repatch.patch(Plug.Conn, :read_body, fn conn, _opts -> {:more, "partial", conn} end) + + conn = + Plug.Test.conn(:post, "/?offset=-1", "body") + |> put_req_header("content-type", "application/json") + |> call_serve_shape_plug(ctx) + + assert conn.status == 413 + assert %{"error" => "Request body too large"} = Jason.decode!(conn.resp_body) + end + + test "returns 400 for body read failure without crashing", ctx do + Repatch.patch(Plug.Conn, :read_body, fn _conn, _opts -> {:error, :timeout} end) + + conn = + Plug.Test.conn(:post, "/?offset=-1", "body") + |> put_req_header("content-type", "application/json") + |> call_serve_shape_plug(ctx) + + assert conn.status == 400 + assert %{"error" => "Failed to read request body"} = Jason.decode!(conn.resp_body) + end end describe "stack not ready" do From 9725a13400c7ee20c35abb50da2594110e7d2903 Mon Sep 17 00:00:00 2001 From: Oleksii Sholik Date: Mon, 2 Mar 2026 11:52:35 +0100 Subject: [PATCH 6/6] ci: allow claude[bot] to trigger review workflow Co-Authored-By: Claude Opus 4.6 --- .github/workflows/claude-code-review.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index e9e7431ee7..c3b8f26fb2 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -114,4 +114,5 @@ jobs: Start by reading `.claude/commands/pr-review.md`, then follow its phases to conduct the review. + allowed_bots: 'claude[bot]' claude_args: '--allowedTools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(gh api:*),Bash(cat:*),Read,Glob,Grep"'