From a754c0fdc2e33b3755e6225b08784c815d800cc7 Mon Sep 17 00:00:00 2001
From: Marcin Kulik <m@ku1ik.com>
Date: Sun, 21 May 2017 17:43:45 +0200
Subject: [PATCH] 503 + Retry-After

---
 config/config.exs                             |  6 +--
 lib/asciinema.ex                              |  9 +---
 lib/asciinema/png_generator/a2png.ex          | 47 +++++++++++++++----
 web/controllers/asciicast_image_controller.ex | 16 +++++--
 4 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/config/config.exs b/config/config.exs
index a8badfb..2b851c9 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -45,9 +45,9 @@ else
 end
 
 config :asciinema, :png_generator, Asciinema.PngGenerator.A2png
-config :asciinema, Asciinema.PngGenerator.A2png, bin_path: "./a2png/a2png.sh"
-
-config :porcelain, goon_warn_if_missing: false
+config :asciinema, Asciinema.PngGenerator.A2png,
+  bin_path: System.get_env("A2PNG_BIN_PATH") || "./a2png/a2png.sh",
+  pool_size: String.to_integer(System.get_env("A2PNG_POOL_SIZE") || "2")
 
 # Import environment specific config. This must remain at the bottom
 # of this file so it overrides the configuration defined above.
diff --git a/lib/asciinema.ex b/lib/asciinema.ex
index 6cad493..dfbc22c 100644
--- a/lib/asciinema.ex
+++ b/lib/asciinema.ex
@@ -14,7 +14,7 @@ defmodule Asciinema do
       supervisor(Asciinema.Endpoint, []),
       # Start your own worker by calling: Asciinema.Worker.start_link(arg1, arg2, arg3)
       # worker(Asciinema.Worker, [arg1, arg2, arg3]),
-      :poolboy.child_spec(:worker, poolboy_config(), []),
+      :poolboy.child_spec(:worker, Asciinema.PngGenerator.A2png.poolboy_config(), []),
     ]
 
     # See http://elixir-lang.org/docs/stable/elixir/Supervisor.html
@@ -29,11 +29,4 @@ defmodule Asciinema do
     Asciinema.Endpoint.config_change(changed, removed)
     :ok
   end
-
-  defp poolboy_config do
-    [{:name, {:local, :worker}},
-     {:worker_module, Asciinema.PngGenerator.A2png},
-     {:size, 2},
-     {:max_overflow, 0}]
-  end
 end
diff --git a/lib/asciinema/png_generator/a2png.ex b/lib/asciinema/png_generator/a2png.ex
index f905f1d..01498d7 100644
--- a/lib/asciinema/png_generator/a2png.ex
+++ b/lib/asciinema/png_generator/a2png.ex
@@ -3,16 +3,31 @@ defmodule Asciinema.PngGenerator.A2png do
   use GenServer
   alias Asciinema.Asciicast
 
-  @result_timeout 30000
+  @pool_name :worker
   @acquire_timeout 5000
+  @a2png_timeout 30000
+  @result_timeout 35000
 
   def generate(%Asciicast{} = asciicast) do
     {:ok, tmp_dir_path} = Briefly.create(directory: true)
 
-    :poolboy.transaction(
-      :worker,
-      &GenServer.call(&1, {:gen_png, asciicast, tmp_dir_path}, @result_timeout), @acquire_timeout
-    )
+    try do
+      :poolboy.transaction(
+        @pool_name,
+        (fn pid ->
+          try do
+            GenServer.call(pid, {:generate, asciicast, tmp_dir_path}, @result_timeout)
+          catch
+            :exit, {:timeout, _} ->
+              {:error, :timeout}
+          end
+        end),
+        @acquire_timeout
+      )
+    catch
+      :exit, {:timeout, _} ->
+        {:error, :busy}
+    end
   end
 
   # GenServer API
@@ -25,11 +40,18 @@ defmodule Asciinema.PngGenerator.A2png do
     {:ok, nil}
   end
 
-  def handle_call({:gen_png, asciicast, tmp_dir_path}, _from, state) do
-    {:reply, do_gen(asciicast, tmp_dir_path), state}
+  def handle_call({:generate, asciicast, tmp_dir_path}, _from, state) do
+    {:reply, do_generate(asciicast, tmp_dir_path), state}
   end
 
-  defp do_gen(asciicast, tmp_dir_path) do
+  def poolboy_config do
+    [{:name, {:local, @pool_name}},
+     {:worker_module, __MODULE__},
+     {:size, pool_size()},
+     {:max_overflow, 0}]
+  end
+
+  defp do_generate(asciicast, tmp_dir_path) do
     path = Asciicast.json_store_path(asciicast)
     json_path = Path.join(tmp_dir_path, "tmp.json")
     png_path = Path.join(tmp_dir_path, "tmp.png")
@@ -37,7 +59,8 @@ defmodule Asciinema.PngGenerator.A2png do
 
     with {:ok, file} <- file_store().open(path),
          {:ok, _} <- :file.copy(file, json_path),
-         %{status: 0} <- Porcelain.exec(bin_path(), [json_path, png_path, snapshot_at]) do
+         process <- Porcelain.spawn(bin_path(), [json_path, png_path, snapshot_at]),
+         {:ok, %{status: 0}} <- Porcelain.Process.await(process, @a2png_timeout) do
       {:ok, png_path}
     else
       otherwise ->
@@ -45,10 +68,14 @@ defmodule Asciinema.PngGenerator.A2png do
     end
   end
 
-  def bin_path do
+  defp bin_path do
     Keyword.get(Application.get_env(:asciinema, __MODULE__), :bin_path)
   end
 
+  defp pool_size do
+    Keyword.get(Application.get_env(:asciinema, __MODULE__), :pool_size)
+  end
+
   defp file_store do
     Application.get_env(:asciinema, :file_store)
   end
diff --git a/web/controllers/asciicast_image_controller.ex b/web/controllers/asciicast_image_controller.ex
index 376ca1d..5da3a6c 100644
--- a/web/controllers/asciicast_image_controller.ex
+++ b/web/controllers/asciicast_image_controller.ex
@@ -5,11 +5,17 @@ defmodule Asciinema.AsciicastImageController do
 
   def show(conn, %{"id" => id} = _params) do
     asciicast = Repo.one!(Asciicast.by_id_or_secret_token(id))
-    {:ok, png_path} = PngGenerator.generate(asciicast)
 
-    conn
-    |> put_resp_header("content-type", MIME.path(png_path))
-    |> send_file(200, png_path)
-    |> halt
+    case PngGenerator.generate(asciicast) do
+      {:ok, png_path} ->
+        conn
+        |> put_resp_header("content-type", MIME.path(png_path))
+        |> send_file(200, png_path)
+        |> halt
+      {:error, :busy} ->
+        conn
+        |> put_resp_header("retry-after", "5")
+        |> send_resp(503, "")
+    end
   end
 end