From fee808bc623f3ae3fa1e08b86ae6014a7f19b791 Mon Sep 17 00:00:00 2001 From: Jamil Date: Mon, 3 Mar 2025 21:21:39 +0000 Subject: [PATCH] chore(portal): Log error for unknown channel messages (#8299) Instead of crashing, it would make sense to log these and let the connected entity maintain its WebSocket connection. This should never happen in practice if we maintain our version compatibility matrix properly, but it will help reduce the blast radius of a channel message bug that happens to slip out into the wild. Fixes #4679 --- elixir/apps/api/lib/api/client/channel.ex | 7 +++++++ elixir/apps/api/lib/api/gateway/channel.ex | 7 +++++++ elixir/apps/api/lib/api/relay/channel.ex | 9 +++++++++ elixir/apps/api/test/api/client/channel_test.exs | 7 +++++++ elixir/apps/api/test/api/gateway/channel_test.exs | 7 +++++++ elixir/apps/api/test/api/relay/channel_test.exs | 7 +++++++ 6 files changed, 44 insertions(+) diff --git a/elixir/apps/api/lib/api/client/channel.ex b/elixir/apps/api/lib/api/client/channel.ex index bc171a9dd..fefd1e56b 100644 --- a/elixir/apps/api/lib/api/client/channel.ex +++ b/elixir/apps/api/lib/api/client/channel.ex @@ -900,6 +900,13 @@ defmodule API.Client.Channel do end end + # Catch-all for unknown messages + def handle_in(message, payload, socket) do + Logger.error("Unknown client message", message: message, payload: payload) + + {:reply, {:error, %{reason: :unknown_message}}, socket} + end + defp select_relays(socket, except_ids \\ []) do {:ok, relays} = Relays.all_connected_relays_for_account(socket.assigns.subject.account, except_ids) diff --git a/elixir/apps/api/lib/api/gateway/channel.ex b/elixir/apps/api/lib/api/gateway/channel.ex index 200c2eacd..7e8783ca2 100644 --- a/elixir/apps/api/lib/api/gateway/channel.ex +++ b/elixir/apps/api/lib/api/gateway/channel.ex @@ -657,6 +657,13 @@ defmodule API.Gateway.Channel do end end + # Catch-all for unknown messages + def handle_in(message, payload, socket) do + Logger.error("Unknown gateway message", message: message, payload: payload) + + {:reply, {:error, %{reason: :unknown_message}}, socket} + end + defp encode_ref(socket, tuple) do ref = tuple diff --git a/elixir/apps/api/lib/api/relay/channel.ex b/elixir/apps/api/lib/api/relay/channel.ex index 4c14f957e..7acb97ef4 100644 --- a/elixir/apps/api/lib/api/relay/channel.ex +++ b/elixir/apps/api/lib/api/relay/channel.ex @@ -2,6 +2,7 @@ defmodule API.Relay.Channel do use API, :channel alias Domain.Relays require OpenTelemetry.Tracer + require Logger @impl true def join("relay", %{"stamp_secret" => stamp_secret}, socket) do @@ -45,4 +46,12 @@ defmodule API.Relay.Channel do {:noreply, socket} end end + + # Catch-all for unknown messages + @impl true + def handle_in(message, payload, socket) do + Logger.error("Unknown relay message", message: message, payload: payload) + + {:reply, {:error, %{reason: :unknown_message}}, socket} + end end diff --git a/elixir/apps/api/test/api/client/channel_test.exs b/elixir/apps/api/test/api/client/channel_test.exs index f6366aa2c..afa4d6411 100644 --- a/elixir/apps/api/test/api/client/channel_test.exs +++ b/elixir/apps/api/test/api/client/channel_test.exs @@ -2384,4 +2384,11 @@ defmodule API.Client.ChannelTest do assert client.id == client_id end end + + describe "handle_in/3 for unknown messages" do + test "it doesn't crash", %{socket: socket} do + ref = push(socket, "unknown_message", %{}) + assert_reply ref, :error, %{reason: :unknown_message} + end + end end diff --git a/elixir/apps/api/test/api/gateway/channel_test.exs b/elixir/apps/api/test/api/gateway/channel_test.exs index 713b01a1a..e3b0b1af3 100644 --- a/elixir/apps/api/test/api/gateway/channel_test.exs +++ b/elixir/apps/api/test/api/gateway/channel_test.exs @@ -1181,4 +1181,11 @@ defmodule API.Gateway.ChannelTest do assert upserted_activity.account_id == account.id end end + + describe "handle_in/3 for unknown messages" do + test "it doesn't crash", %{socket: socket} do + ref = push(socket, "unknown_message", %{}) + assert_reply ref, :error, %{reason: :unknown_message} + end + end end diff --git a/elixir/apps/api/test/api/relay/channel_test.exs b/elixir/apps/api/test/api/relay/channel_test.exs index 687f990f0..cd4dc5922 100644 --- a/elixir/apps/api/test/api/relay/channel_test.exs +++ b/elixir/apps/api/test/api/relay/channel_test.exs @@ -52,4 +52,11 @@ defmodule API.Relay.ChannelTest do assert_push "init", %{} end end + + describe "handle_in/3 for unknown messages" do + test "it doesn't crash", %{socket: socket} do + ref = push(socket, "unknown_message", %{}) + assert_reply ref, :error, %{reason: :unknown_message} + end + end end