Skip to content

Commit

Permalink
Fix CMS paragraph order being scrambled
Browse files Browse the repository at this point in the history
The issue was that `async_with_timeout`, which accepts a list of
functions, was implemented on top of `yield_or_default_many`, which
accepts a map where the values are functions. The map values were being
collected back into a list, but since maps are not ordered, this result
list would not have the same ordering as the input list.

Because the previous approach resulted in this data flow:

list -> map -> list -> Task.yield_many -> list -> map -> list

We can simplify to this:

list -> Task.yield_many -> list
  • Loading branch information
digitalcora committed Dec 24, 2019
1 parent 171281e commit 1798876
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 17 deletions.
24 changes: 12 additions & 12 deletions apps/util/lib/util.ex
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,11 @@ defmodule Util do
when is_list(functions) and is_atom(module) do
functions
|> Enum.map(&Task.async/1)
|> Task.yield_many(timeout)
|> Enum.with_index()
|> Map.new(fn {task, idx} -> {task, {idx, default}} end)
|> yield_or_default_many(module, timeout)
|> Enum.map(fn {_, result} -> result end)
|> Enum.map(fn {{task, result}, index} ->
task_result_or_default(result, default, task, module, index)
end)
end

@doc """
Expand All @@ -154,7 +155,7 @@ defmodule Util do
def yield_or_default(%Task{} = task, timeout, default, module) when is_atom(module) do
task
|> Task.yield(timeout)
|> task_result_or_default(default, task, {"", module})
|> task_result_or_default(default, task, module, "")
end

@doc """
Expand All @@ -174,29 +175,28 @@ defmodule Util do
{atom, any}
defp do_yield_or_default_many({%Task{} = task, result}, task_map, module) do
{key, default} = Map.get(task_map, task)
{key, task_result_or_default(result, default, task, {key, module})}
{key, task_result_or_default(result, default, task, module, key)}
end

@spec task_result_or_default({:ok, any} | {:exit, term} | nil, any, Task.t(), {any, atom}) ::
any
defp task_result_or_default({:ok, result}, _default, %Task{}, _module) do
@spec task_result_or_default({:ok, any} | {:exit, term} | nil, any, Task.t(), atom, any) :: any
defp task_result_or_default({:ok, result}, _default, _task, _module, _key) do
result
end

defp task_result_or_default({:exit, _reason}, default, %Task{}, {key, module}) do
defp task_result_or_default({:exit, reason}, default, _task, module, key) do
_ =
Logger.warn(
"module=#{module} " <>
"key=#{key} " <>
"error=async_error " <>
"error_type=timeout " <>
"Async task exited unexpectedly. Returning: #{inspect(default)}"
"Async task exited for reason: #{inspect(reason)} -- Defaulting to: #{inspect(default)}"
)

default
end

defp task_result_or_default(nil, default, %Task{} = task, {key, module}) do
defp task_result_or_default(nil, default, %Task{} = task, module, key) do
case Task.shutdown(task, :brutal_kill) do
{:ok, result} ->
result
Expand All @@ -208,7 +208,7 @@ defmodule Util do
"key=#{key} " <>
"error=async_error " <>
"error_type=timeout " <>
"async task timed out. Returning: #{inspect(default)}"
"Async task timed out -- Defaulting to: #{inspect(default)}"
)

default
Expand Down
2 changes: 1 addition & 1 deletion apps/util/test/async_assign_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ defmodule Util.AsyncAssignTest do
assert log =~ "module=#{__MODULE__}"
assert log =~ "key=hello"
assert log =~ "error=async_error"
assert log =~ "async task timed out"
assert log =~ "Async task timed out"
end
end
23 changes: 19 additions & 4 deletions apps/util/test/util_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,21 @@ defmodule UtilTest do
assert async_with_timeout([fn -> 5 end, fn -> 6 end], nil, __MODULE__) == [5, 6]
end

test "returns results in the same order functions were passed in" do
results = Enum.to_list(0..99)

functions =
Enum.map(
results,
&fn ->
:timer.sleep(&1)
&1
end
)

assert async_with_timeout(functions, nil, __MODULE__) == results
end

test "returns the default for a task that runs too long and logs a warning" do
log =
capture_log(fn ->
Expand All @@ -327,7 +342,7 @@ defmodule UtilTest do
) == [5, :default]
end)

assert log =~ "async task timed out"
assert log =~ "Async task timed out"
end
end

Expand Down Expand Up @@ -377,9 +392,9 @@ defmodule UtilTest do
}
end)

assert log =~ "Returning: :long_default"
assert log =~ "task exited unexpectedly"
refute log =~ "Returning: :short_default"
assert log =~ "Defaulting to: :long_default"
assert log =~ "exited for reason: :killed"
refute log =~ "Defaulting to: :short_default"
end
end

Expand Down

0 comments on commit 1798876

Please sign in to comment.