From 303b8da79a28282d87a2909056829e24f3d34a2a Mon Sep 17 00:00:00 2001 From: Tyler Young Date: Thu, 19 Mar 2026 13:06:44 -0500 Subject: [PATCH] fix (timeout errors): Standardize on `{:error, :timeout}` The `can't block js workers` test was flaky locally for me because sometimes a task would result in `{:error, "Call timed out."}` and others it would be `{:error, :timeout}`. The string version was coming from the NodeJS.Supervisor, while the atom came from NodeJS.Worker. Since the atom version is more idiomatic to Elixir, I thought it'd be good to standardize on that. --- lib/nodejs/supervisor.ex | 7 ++++--- test/nodejs_test.exs | 14 +++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/nodejs/supervisor.ex b/lib/nodejs/supervisor.ex index a39d9b8..6688296 100644 --- a/lib/nodejs/supervisor.ex +++ b/lib/nodejs/supervisor.ex @@ -39,7 +39,7 @@ defmodule NodeJS.Supervisor do GenServer.call(pid, {module, args, [binary: binary, timeout: timeout, esm: esm]}, timeout) catch :exit, {:timeout, _} -> - {:error, "Call timed out."} + {:error, :timeout} :exit, error -> {:error, {:node_js_worker_exit, error}} @@ -69,7 +69,7 @@ defmodule NodeJS.Supervisor do run_in_transaction(module, args, opts) catch :exit, {:timeout, _} -> - {:error, "Call timed out."} + {:error, :timeout} end end @@ -78,7 +78,8 @@ defmodule NodeJS.Supervisor do |> call(args, opts) |> case do {:ok, result} -> result - {:error, message} -> raise NodeJS.Error, message: message + {:error, message} when is_binary(message) -> raise NodeJS.Error, message: message + {:error, :timeout} -> raise NodeJS.Error, message: "Call timed out." end end diff --git a/test/nodejs_test.exs b/test/nodejs_test.exs index 713e5f3..235b9ae 100644 --- a/test/nodejs_test.exs +++ b/test/nodejs_test.exs @@ -164,10 +164,10 @@ defmodule NodeJS.Test do assert_receive :received_timeout_3, 10 assert_receive :received_timeout_4, 10 - assert {:error, "Call timed out."} = Task.await(task1) - assert {:error, "Call timed out."} = Task.await(task2) - assert {:error, "Call timed out."} = Task.await(task3) - assert {:error, "Call timed out."} = Task.await(task4) + assert {:error, :timeout} = Task.await(task1) + assert {:error, :timeout} = Task.await(task2) + assert {:error, :timeout} = Task.await(task3) + assert {:error, :timeout} = Task.await(task4) # We should still get an answer here, before the timeout assert {:ok, 1115} = NodeJS.call("slow-async-echo", [1115, 1]) @@ -176,7 +176,7 @@ defmodule NodeJS.Test do describe "overriding call timeout" do test "works, and you can tell because the slow function will time out" do - assert {:error, "Call timed out."} = NodeJS.call("slow-async-echo", [1111], timeout: 0) + assert {:error, :timeout} = NodeJS.call("slow-async-echo", [1111], timeout: 0) assert_raise NodeJS.Error, fn -> NodeJS.call!("slow-async-echo", [1111], timeout: 0) end assert {:ok, 1111} = NodeJS.call("slow-async-echo", [1111]) end @@ -190,9 +190,9 @@ defmodule NodeJS.Test do describe "Implementation details shouldn't leak:" do test "Timeouts do not send stray messages to calling process" do - assert {:error, "Call timed out."} = NodeJS.call("slow-async-echo", [1111], timeout: 0) + assert {:error, :timeout} = NodeJS.call("slow-async-echo", [1111], timeout: 0) - refute_receive {_ref, {:error, "Call timed out."}}, 50 + refute_receive {_ref, {:error, :timeout}}, 50 end test "Crashes do not bring down the calling process" do