runtime checks for socket access in async functions (#3161)

* runtime checks for socket access in async functions

* use stacktrace for runtime case

* Update lib/phoenix_live_view/async.ex

Co-authored-by: José Valim <[email protected]>

* only use Function.info if enable_expensive_runtime_checks is true

* add warning for accessing an assigns map

* set enable_expensive_runtime_checks in test config

* add eval_quoted test variants

---------

Co-authored-by: José Valim <[email protected]>

config/config.exs

@@ -2,8 +2,6 @@ import Config
 
 config :phoenix, :json_library, Jason
 config :phoenix, :trim_on_html_eex_engine, false
-config :logger, :level, :debug
-config :logger, :backends, []
 
 if Mix.env() == :dev do
   esbuild = fn args ->
@@ -21,3 +19,5 @@ if Mix.env() == :dev do
     cdn: esbuild.(~w(--format=iife --target=es2016 --global-name=LiveView --outfile=../priv/static/phoenix_live_view.js)),
     cdn_min: esbuild.(~w(--format=iife --target=es2016 --global-name=LiveView --minify --outfile=../priv/static/phoenix_live_view.min.js))
 end
+
+import_config "#{config_env()}.exs"

config/dev.exs

@@ -0,0 +1 @@
+import Config

config/e2e.exs

@@ -0,0 +1,3 @@
+import Config
+
+config :logger, :level, :error

config/test.exs

@@ -0,0 +1,6 @@
+import Config
+
+config :logger, :level, :debug
+config :logger, :backends, []
+
+config :phoenix_live_view, enable_expensive_runtime_checks: true

lib/phoenix_live_view/async.ex

@@ -3,44 +3,73 @@ defmodule Phoenix.LiveView.Async do
 
   alias Phoenix.LiveView.{AsyncResult, Socket, Channel}
 
-  defp validate_function_env(func, op, env) do
-    warn =
-      # TODO: Remove conditional once we require Elixir v1.14+
-      if Version.match?(System.version(), ">= 1.14.0") do
-        fn msg, env -> IO.warn(msg, env) end
-      else
-        fn msg, _env -> IO.warn(msg) end
-      end
+  defp warn_socket_access(op, warn) do
+    warn.("""
+    you are accessing the LiveView Socket inside a function given to #{op}.
 
-    # prevent false positives, for example
-    # start_async(socket, :foo, function_that_returns_the_anonymous_function(socket))
-    if match?({:&, _, _}, func) or match?({:fn, _, _}, func) do
-      Macro.prewalk(func, fn
-        {:socket, meta, nil} ->
-          warn.(
-            """
-            you are accessing the LiveView Socket inside a function given to #{op}.
+    This is an expensive operation because the whole socket is copied to the new process.
+
+    Instead of:
 
-            This is an expensive operation because the whole socket is copied to the new process.
+        #{op}(socket, :key, fn ->
+          do_something(socket.assigns.my_assign)
+        end)
 
-            Instead of:
+    You should do:
 
-                #{op}(socket, :key, fn ->
-                  do_something(socket.assigns.my_assign)
-                end)
+        my_assign = socket.assigns.my_assign
 
-            You should do:
+        #{op}(socket, :key, fn ->
+          do_something(my_assign)
+        end)
+
+    For more information, see https://hexdocs.pm/elixir/1.16.1/process-anti-patterns.html#sending-unnecessary-data.
+    """)
+  end
 
-                my_assign = socket.assigns.my_assign
+  # this is not private to prevent the unused function warning as we only
+  # call this function when enable_expensive_runtime_checks is set
+  def warn_assigns_access(op, warn) do
+    warn.("""
+    you are accessing an assigns map inside a function given to #{op}.
 
-                #{op}(socket, :key, fn ->
-                  do_something(my_assign)
-                end)
+    This is an expensive operation because the whole map is copied to the new process.
 
-            For more information, see https://hexdocs.pm/elixir/1.16.1/process-anti-patterns.html#sending-unnecessary-data.
-            """,
-            Keyword.take(meta, [:line, :column]) ++ [line: env.line, file: env.file]
-          )
+    Instead of:
+
+        #{op}(socket, :key, fn ->
+          do_something(assigns.my_assign)
+        end)
+
+    You should do:
+
+        my_assign = assigns.my_assign
+
+        #{op}(socket, :key, fn ->
+          do_something(my_assign)
+        end)
+
+    For more information, see https://hexdocs.pm/elixir/1.16.1/process-anti-patterns.html#sending-unnecessary-data.
+    """)
+  end
+
+  defp validate_function_env(func, op, env) do
+    # prevent false positives, for example
+    # start_async(socket, :foo, function_that_returns_the_anonymous_function(socket))
+    if match?({:&, _, _}, func) or match?({:fn, _, _}, func) do
+      Macro.prewalk(func, fn
+        {:socket, meta, _} ->
+          warn_socket_access(op, fn msg ->
+            # TODO: Remove conditional once we require Elixir v1.14+
+            meta =
+              if Version.match?(System.version(), ">= 1.14.0") do
+                Keyword.take(meta, [:line, :column]) ++ [line: env.line, file: env.file]
+              else
+                Macro.Env.stacktrace(env)
+              end
+
+            IO.warn(msg, meta)
+          end)
 
         other ->
           other
@@ -50,20 +79,47 @@ defmodule Phoenix.LiveView.Async do
     :ok
   end
 
+  if Application.compile_env(:phoenix_live_view, :enable_expensive_runtime_checks, false) do
+    defp validate_function_env(func, op) do
+      {:env, variables} = Function.info(func, :env)
+
+      cond do
+        Enum.any?(variables, &match?(%Phoenix.LiveView.Socket{}, &1)) ->
+          warn_socket_access(op, fn msg -> IO.warn(msg) end)
+
+        Enum.any?(variables, &match?(%{__changed__: _}, &1)) ->
+          warn_assigns_access(op, fn msg -> IO.warn(msg) end)
+
+        true ->
+          :ok
+      end
+    end
+  else
+    defp validate_function_env(_func, _op), do: :ok
+  end
+
   def start_async(socket, key, func, opts, env) do
     validate_function_env(func, :start_async, env)
 
     quote do
-      Phoenix.LiveView.Async.run_async_task(
+      Phoenix.LiveView.Async.start_async(
         unquote(socket),
         unquote(key),
         unquote(func),
-        :start,
         unquote(opts)
       )
     end
   end
 
+  def start_async(%Socket{} = socket, key, func, opts) when is_function(func, 0) do
+    # runtime check
+    if Phoenix.LiveView.connected?(socket) do
+      validate_function_env(func, :start_async)
+    end
+
+    run_async_task(socket, key, func, :start, opts)
+  end
+
   def assign_async(socket, key_or_keys, func, opts, env) do
     validate_function_env(func, :assign_async, env)
 
@@ -80,6 +136,11 @@ defmodule Phoenix.LiveView.Async do
   def assign_async(%Socket{} = socket, key_or_keys, func, opts)
       when (is_atom(key_or_keys) or is_list(key_or_keys)) and
              is_function(func, 0) do
+    # runtime check
+    if Phoenix.LiveView.connected?(socket) do
+      validate_function_env(func, :assign_async)
+    end
+
     keys = List.wrap(key_or_keys)
 
     # verifies result inside task

test/phoenix_live_view/async_test.exs

@@ -0,0 +1,126 @@
+defmodule Phoenix.LiveView.AsyncTest do
+  # run with async: false to prevent other messages from being captured
+  use ExUnit.Case, async: false
+
+  import ExUnit.CaptureIO
+
+  describe "async operations - eval_quoted" do
+    for fun <- [:assign_async, :start_async] do
+      test "warns when passing socket to #{fun} function" do
+        warnings =
+          capture_io(:stderr, fn ->
+            fun = unquote(fun)
+
+            Code.eval_quoted(quote do
+              require Phoenix.LiveView
+
+              socket = %Phoenix.LiveView.Socket{assigns: %{__changed__: %{}, bar: :baz}}
+
+              Phoenix.LiveView.unquote(fun)(socket, :foo, fn ->
+                socket.assigns.bar
+              end)
+            end)
+          end)
+
+        assert warnings =~
+                 "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}"
+      end
+
+      test "does not warn when accessing socket outside of function passed to #{fun}" do
+        warnings =
+          capture_io(:stderr, fn ->
+            fun = unquote(fun)
+
+            Code.eval_quoted(quote do
+              require Phoenix.LiveView
+
+              socket = %Phoenix.LiveView.Socket{assigns: %{__changed__: %{}, bar: :baz}}
+              bar = socket.assigns.bar
+
+              Phoenix.LiveView.unquote(fun)(socket, :foo, fn ->
+                bar
+              end)
+            end)
+          end)
+
+        refute warnings =~
+                 "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}"
+      end
+    end
+  end
+
+  describe "async operations" do
+    for fun <- [:assign_async, :start_async] do
+      test "warns when passing socket to #{fun} function", %{test: test} do
+        warnings =
+          capture_io(:stderr, fn ->
+            defmodule Module.concat(AssignAsyncSocket, "Test#{:erlang.phash2(test)}") do
+              use Phoenix.LiveView
+
+              def mount(_params, _session, socket) do
+                {:ok, unquote(fun)(socket, :foo, fn ->
+                  do_something(socket.assigns)
+                end)}
+              end
+
+              defp do_something(_socket), do: :ok
+            end
+          end)
+
+        assert warnings =~
+                 "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}"
+      end
+
+      test "does not warn when accessing socket outside of function passed to #{fun}", %{test: test} do
+        warnings =
+          capture_io(:stderr, fn ->
+            defmodule Module.concat(AssignAsyncSocket, "Test#{:erlang.phash2(test)}") do
+              use Phoenix.LiveView
+
+              def mount(_params, _session, socket) do
+                socket = assign(socket, :foo, :bar)
+                foo = socket.assigns.foo
+
+                {:ok, unquote(fun)(socket, :foo, fn ->
+                  do_something(foo)
+                end)}
+              end
+
+              defp do_something(assigns), do: :ok
+            end
+          end)
+
+        refute warnings =~
+                 "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}"
+      end
+
+      test "does not warn when argument is not a function (#{fun})", %{test: test} do
+        warnings =
+          capture_io(:stderr, fn ->
+            defmodule Module.concat(AssignAsyncSocket, "Test#{:erlang.phash2(test)}") do
+              use Phoenix.LiveView
+
+              def mount(_params, _session, socket) do
+                socket = assign(socket, :foo, :bar)
+
+                {:ok, unquote(fun)(socket, :foo, function_that_returns_the_func(socket))}
+              end
+
+              defp function_that_returns_the_func(socket) do
+                foo = socket.assigns.foo
+
+                fn ->
+                  do_something(foo)
+                end
+              end
+
+              defp do_something(assigns), do: :ok
+            end
+          end)
+
+        refute warnings =~
+                 "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}"
+      end
+    end
+  end
+end