diff --git a/services/app/apps/codebattle/lib/codebattle/application.ex b/services/app/apps/codebattle/lib/codebattle/application.ex index 4861926c2..c89e77b10 100644 --- a/services/app/apps/codebattle/lib/codebattle/application.ex +++ b/services/app/apps/codebattle/lib/codebattle/application.ex @@ -46,7 +46,8 @@ defmodule Codebattle.Application do %{ id: Codebattle.Chat.Lobby, start: {Codebattle.Chat, :start_link, [:lobby, %{message_ttl: to_timeout(hour: 8)}]} - } + }, + {Task.Supervisor, name: Codebattle.CheckerTaskSupervisor} ] Supervisor.start_link(children, diff --git a/services/app/apps/codebattle/lib/codebattle/code_check/checker.ex b/services/app/apps/codebattle/lib/codebattle/code_check/checker.ex index c14545dfe..43275b73c 100644 --- a/services/app/apps/codebattle/lib/codebattle/code_check/checker.ex +++ b/services/app/apps/codebattle/lib/codebattle/code_check/checker.ex @@ -7,8 +7,9 @@ defmodule Codebattle.CodeCheck.Checker do require Logger - @spec call(Codebattle.Task.t(), String.t(), String.t()) :: CodeCheck.check_result() - def call(task, solution_text, lang_slug) do + @spec execute_check_synchronously(Codebattle.Task.t(), String.t(), String.t()) :: + CodeCheck.check_result() + def execute_check_synchronously(task, solution_text, lang_slug) do lang_meta = Languages.meta(lang_slug) token = @@ -19,13 +20,51 @@ defmodule Codebattle.CodeCheck.Checker do lang_meta: lang_meta, executor: get_executor() }) - |> execute() - |> parse_output() + |> do_execute_token() + |> do_parse_output() token.result end - defp execute(token) do + def check_solution( + caller_pid, + task, + solution_text, + lang_slug, + original_user_for_callback, + original_editor_text_for_callback, + original_editor_lang_for_callback + ) do + Task.Supervisor.async_nolink(Codebattle.CheckerTaskSupervisor, fn -> + try + lang_meta = Languages.meta(lang_slug) + + token_result = + Token + |> struct(%{ + task: task, + solution_text: solution_text, + lang_meta: lang_meta, + executor: get_executor() + }) + |> do_execute_token() + |> do_parse_output() + + send(caller_pid, {:code_check_result, token_result.result, original_user_for_callback, original_editor_text_for_callback, original_editor_lang_for_callback}) + catch + kind, reason -> + stacktrace = __STACKTRACE__ + + Logger.error( + "Async check solution failed: #{kind} - #{inspect(reason)} - #{inspect(stacktrace)}" + ) + + send(caller_pid, {:code_check_error, {kind, reason, stacktrace}, original_user_for_callback, original_editor_text_for_callback, original_editor_lang_for_callback}) + end + end) + end + + defp do_execute_token(token) do {execution_time, new_token} = :timer.tc(fn -> token.executor.call(token) end) execution_time_msec = div(execution_time, 1_000) @@ -36,7 +75,7 @@ defmodule Codebattle.CodeCheck.Checker do %{new_token | execution_time_msec: execution_time_msec} end - defp parse_output(token) do + defp do_parse_output(token) do %{token | result: OutputParser.call(token)} end diff --git a/services/app/apps/codebattle/lib/codebattle/game/engine.ex b/services/app/apps/codebattle/lib/codebattle/game/engine.ex index df26edb3c..0df204282 100644 --- a/services/app/apps/codebattle/lib/codebattle/game/engine.ex +++ b/services/app/apps/codebattle/lib/codebattle/game/engine.ex @@ -144,6 +144,8 @@ defmodule Codebattle.Game.Engine do end end + alias Codebattle.CodeCheck.Checker + def check_result(game, params) do %{user: user, editor_text: editor_text, editor_lang: editor_lang} = params @@ -159,61 +161,20 @@ defmodule Codebattle.Game.Engine do user_id: user.id }) - check_result = CodeCheck.check_solution(game.task, editor_text, editor_lang) - - # TODO: maybe drop editor_text here - Codebattle.PubSub.broadcast("game:check_completed", %{ - game: game, - user_id: user.id, - check_result: check_result - }) - - Game.Server.update_playbook(game.id, :check_complete, %{ - id: user.id, - check_result: check_result, - editor_text: editor_text, - editor_lang: editor_lang - }) + game_server_pid_meta = Codebattle.Game.Server.server_name(game.id) - case check_result.status do - "ok" -> - {:ok, {old_game_state, new_game}} = - fire_transition(game.id, :check_success, %{ - id: user.id, - check_result: check_result, - editor_text: editor_text, - editor_lang: editor_lang - }) + :ok = + Checker.check_solution( + game_server_pid_meta, + game.task, + editor_text, + editor_lang, + user, + editor_text, + editor_lang + ) - case {old_game_state, new_game.state} do - {"playing", "game_over"} -> - Game.Server.update_playbook(game.id, :game_over, %{ - id: user.id, - lang: editor_lang - }) - - Codebattle.PubSub.broadcast("game:finished", %{game: new_game}) - - {:ok, _game} = store_result!(new_game) - store_playbook_async(new_game) - - {:ok, new_game, %{check_result: check_result, solution_status: true}} - - _ -> - {:ok, new_game, %{check_result: check_result, solution_status: false}} - end - - _ -> - {:ok, {_old_game_state, new_game}} = - fire_transition(game.id, :check_failure, %{ - id: user.id, - check_result: check_result, - editor_text: editor_text, - editor_lang: editor_lang - }) - - {:ok, new_game, %{check_result: check_result, solution_status: false}} - end + :ok end def give_up(game, user) do diff --git a/services/app/apps/codebattle/lib/codebattle/game/server.ex b/services/app/apps/codebattle/lib/codebattle/game/server.ex index 3521d71e8..25a922bd0 100644 --- a/services/app/apps/codebattle/lib/codebattle/game/server.ex +++ b/services/app/apps/codebattle/lib/codebattle/game/server.ex @@ -4,6 +4,7 @@ defmodule Codebattle.Game.Server do use GenServer alias Codebattle.Game + alias Codebattle.Game.Engine alias Codebattle.Playbook require Logger @@ -110,4 +111,73 @@ defmodule Codebattle.Game.Server do end defp server_name(game_id), do: {:via, Registry, {Codebattle.Registry, "game_srv:#{game_id}"}} + + @impl GenServer + def handle_info({:code_check_result, check_result, user, editor_text, editor_lang}, state) do + %{game: game} = state + + Codebattle.PubSub.broadcast("game:check_completed", %{ + game: game, + user_id: user.id, + check_result: check_result + }) + + # Update playbook for the check completion + GenServer.cast(self(), {:update_playbook, :check_complete, %{ + id: user.id, + check_result: check_result, + editor_text: editor_text, # Consider if editor_text is needed here + editor_lang: editor_lang + }}) + + case check_result.status do + "ok" -> + case GenServer.call(self(), {:transition, :check_success, %{id: user.id, check_result: check_result, editor_text: editor_text, editor_lang: editor_lang}}) do + {:ok, {old_game_state, new_game_from_transition}} -> + new_state = %{state | game: new_game_from_transition} + + case {old_game_state, new_game_from_transition.state} do + {"playing", "game_over"} -> + GenServer.cast(self(), {:update_playbook, :game_over, %{id: user.id, lang: editor_lang}}) + Codebattle.PubSub.broadcast("game:finished", %{game: new_game_from_transition}) + Engine.store_result!(new_game_from_transition) + Engine.store_playbook_async(new_game_from_transition) + {:noreply, new_state} + + _ -> + {:noreply, new_state} + end + {:error, reason} -> + Logger.error("Failed to transition game state on check_success: #{inspect(reason)}") + {:noreply, state} + end + + _ -> # Any other status is a failure + case GenServer.call(self(), {:transition, :check_failure, %{id: user.id, check_result: check_result, editor_text: editor_text, editor_lang: editor_lang}}) do + {:ok, {_old_game_state, new_game_from_transition}} -> + new_state = %{state | game: new_game_from_transition} + {:noreply, new_state} + {:error, reason} -> + Logger.error("Failed to transition game state on check_failure: #{inspect(reason)}") + {:noreply, state} + end + end + end + + @impl GenServer + def handle_info({:code_check_error, error_details, user, _editor_text, _editor_lang}, state) do + %{game: game} = state + + Logger.error("Code check failed for game #{game.id}, user #{user.id}: #{inspect(error_details)}") + + Codebattle.PubSub.broadcast("game:check_failed", %{ + game: game, + user_id: user.id, + error: :internal_error # Or more specific error if available + }) + + # Potentially transition game to an error state or allow user to retry + # For now, just log and broadcast + {:noreply, state} + end end diff --git a/services/app/apps/runner/lib/runner/executor.ex b/services/app/apps/runner/lib/runner/executor.ex index 14a751a09..da73e4b86 100644 --- a/services/app/apps/runner/lib/runner/executor.ex +++ b/services/app/apps/runner/lib/runner/executor.ex @@ -128,4 +128,23 @@ defmodule Runner.Executor do to_string(:rand.uniform(10_000_000)) end end + + defp cleanup_temp_directory(path, retries \\ 3) do + try + # Using rm_rf! to ensure an exception is raised on failure, which the catch block will handle. + File.rm_rf!(path) + # Optional: Logger.debug("Successfully cleaned up temporary directory: #{path}") + catch + kind, reason -> + # Log with a clear indication of which attempt failed. + Logger.error("Failed to clean up temporary directory: #{path}, attempt ##{4 - retries}, reason: #{kind} - #{inspect(reason)}") + if retries > 0 do + :timer.sleep(500) # Wait 500ms before retrying. + cleanup_temp_directory(path, retries - 1) + else + Logger.error("Giving up on cleaning temporary directory after multiple retries: #{path}.") + # Consider if further action like notifying an external system is needed on persistent failure. + end + end + end end