From 778dc059473cb7fa10e3451b6853241fbaab529a Mon Sep 17 00:00:00 2001 From: OlafurArason Date: Tue, 5 Sep 2023 15:44:34 +0200 Subject: [PATCH 1/2] chore: use retry because opentok sometimes fails --- .formatter.exs | 1 + lib/ex_opentok/client.ex | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.formatter.exs b/.formatter.exs index d304ff3..8558637 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -1,3 +1,4 @@ [ + import_deps: [:tesla], inputs: ["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"] ] diff --git a/lib/ex_opentok/client.ex b/lib/ex_opentok/client.ex index 22dd4db..779c382 100644 --- a/lib/ex_opentok/client.ex +++ b/lib/ex_opentok/client.ex @@ -4,14 +4,24 @@ defmodule ExOpentok.Client do alias ExOpentok.Token alias ExOpentok.Exception - plug(Tesla.Middleware.Timeout, timeout: 2_000) + plug Tesla.Middleware.Timeout, timeout: 2_000 + + plug Tesla.Middleware.Retry, + delay: 100, + max_retries: 10, + max_delay: 4_000, + should_retry: fn + {:ok, %{status: status}} when status >= 500 -> true + {:ok, _} -> false + {:error, _} -> true + end @moduledoc """ - Wrapper for HTTPotion + Wrapper for Tesla """ @doc """ - HTTP Client's request with HTTPotion. + HTTP Client's request with Tesla. """ def http_request(url, type \\ :get, body \\ nil) do do_http_request(url, type, body) From 2f6a48a21efb55acfb67e89c6ec31f6af8f4a65c Mon Sep 17 00:00:00 2001 From: OlafurArason Date: Tue, 5 Sep 2023 16:24:51 +0200 Subject: [PATCH 2/2] fix: sensible error handling --- lib/ex_opentok.ex | 4 ++ lib/ex_opentok/archive.ex | 28 +++++++++++ lib/ex_opentok/client.ex | 17 +++++-- lib/ex_opentok/session.ex | 54 +++++++++++++++++--- test/ex_opentok_test.exs | 16 +++--- test/test_helper.exs | 101 ++++++++++++++++++++------------------ 6 files changed, 150 insertions(+), 70 deletions(-) diff --git a/lib/ex_opentok.ex b/lib/ex_opentok.ex index e5d7db1..0e3ab94 100644 --- a/lib/ex_opentok.ex +++ b/lib/ex_opentok.ex @@ -31,6 +31,10 @@ defmodule ExOpentok do ExOpentok.Session.init() end + def init! do + ExOpentok.Session.init!() + end + @doc false def version, do: @version diff --git a/lib/ex_opentok/archive.ex b/lib/ex_opentok/archive.ex index 3fe1f08..c5275fa 100644 --- a/lib/ex_opentok/archive.ex +++ b/lib/ex_opentok/archive.ex @@ -81,6 +81,13 @@ defmodule ExOpentok.Archive do |> Client.handle_response() end + def stop!(archive_id) do + case stop(archive_id) do + {:ok, response} -> response + {:error, error} -> raise error + end + end + @doc """ Deleting an archive @@ -96,6 +103,13 @@ defmodule ExOpentok.Archive do |> Client.handle_response() end + def delete!(archive_id) do + case delete(archive_id) do + {:ok, response} -> response + {:error, error} -> raise error + end + end + @doc """ Listing archives @@ -133,9 +147,23 @@ defmodule ExOpentok.Archive do |> Client.handle_response() end + def list!(opts \\ %{offset: 0, count: 1000}) do + case list(opts) do + {:ok, response} -> response + {:error, error} -> raise error + end + end + def find(archive_id) do (ExOpentok.api_url() <> "#{ExOpentok.config(:key)}/archive/#{archive_id}") |> Client.http_request() |> Client.handle_response() end + + def find!(archive_id) do + case find(archive_id) do + {:ok, response} -> response + {:error, error} -> raise error + end + end end diff --git a/lib/ex_opentok/client.ex b/lib/ex_opentok/client.ex index 779c382..b9d4e68 100644 --- a/lib/ex_opentok/client.ex +++ b/lib/ex_opentok/client.ex @@ -72,19 +72,26 @@ defmodule ExOpentok.Client do def handle_response(response) do case response do {:ok, %{status: 200, body: ""}} -> - %{} + {:ok, %{}} {:ok, %{status: 200, body: body}} -> - body |> Poison.decode!() |> handle_data_struct() + {:ok, body |> Poison.decode!() |> handle_data_struct()} {:ok, %{status: 405, body: body}} -> - raise "405 Method not allowed" + {:ok, "405 Method not allowed"} {:ok, response} -> - raise "Error #{response.status} -> ExOpentok query:\n #{inspect(response)}" + {:error, "Error #{response.status} -> ExOpentok query:\n #{inspect(response)}"} {:error, error} -> - raise "Error #{inspect(error)} -> ExOpentok query:\n #{inspect(response)}" + {:error, "Error #{inspect(error)} -> ExOpentok query:\n #{inspect(response)}"} + end + end + + def handle_response!(response) do + case handle_response(response) do + {:ok, output} -> output + {:error, error} -> raise error end end diff --git a/lib/ex_opentok/session.ex b/lib/ex_opentok/session.ex index 1dd26c0..ba6ee40 100644 --- a/lib/ex_opentok/session.ex +++ b/lib/ex_opentok/session.ex @@ -34,6 +34,13 @@ defmodule ExOpentok.Session do |> Client.handle_response() end + def create!(opts) do + case create(opts) do + {:ok, response} -> response + {:error, error} -> raise error + end + end + # https://tokbox.com/developer/rest/#force_mute_stream @spec mute(String.t(), String.t()) :: %{} def mute(session_id, stream_id) do @@ -42,6 +49,13 @@ defmodule ExOpentok.Session do |> Client.handle_response() end + def mute!(session_id, stream_id) do + case mute(session_id, stream_id) do + {:ok, response} -> response + {:error, error} -> raise error + end + end + # https://tokbox.com/developer/rest/#force_mute_session @spec mute_all(String.t(), [String.t()]) :: %{} def mute_all(session_id, excluded_stream_ids \\ []) do @@ -55,23 +69,40 @@ defmodule ExOpentok.Session do |> ExOpentok.Client.handle_response() end + def mute_all!(session_id, excluded_stream_ids) do + case mute_all(session_id, excluded_stream_ids) do + {:ok, response} -> response + {:error, error} -> raise error + end + end + @spec base_session_url(String.t(), String.t()) :: String.t() defp base_session_url(session_id, path \\ "") do "https://api.opentok.com/v2/project/#{ExOpentok.config(:key)}/session/#{session_id}#{path}" end - defp format_response(session) do - Map.merge( - session, - %{ - api_key: ExOpentok.config(:key), - token: Token.generate(session["session_id"]) - } - ) + defp format_response({:ok, session}) do + {:ok, + Map.merge( + session, + %{ + api_key: ExOpentok.config(:key), + token: Token.generate(session["session_id"]) + } + )} end + defp format_response(error), do: error + def init, do: create() |> format_response() + def init! do + case init() do + {:ok, response} -> response + {:error, error} -> raise error + end + end + @doc """ Handle location """ @@ -120,4 +151,11 @@ defmodule ExOpentok.Session do |> Client.http_request() |> Client.handle_response() end + + def delete!(session_id, connection_id) do + case delete(session_id, connection_id) do + {:ok, response} -> response + {:error, error} -> raise error + end + end end diff --git a/test/ex_opentok_test.exs b/test/ex_opentok_test.exs index 5d797ac..cf2ff38 100644 --- a/test/ex_opentok_test.exs +++ b/test/ex_opentok_test.exs @@ -4,8 +4,8 @@ defmodule ExOpentokTest do alias ExOpentok.{Archive, Client, Exception, Session, Token} @api_key ExOpentok.config(:key) - @session ExOpentok.init() - @list ExOpentok.Archive.list() + @session ExOpentok.init!() + @list ExOpentok.Archive.list!() # ARCHIVE @@ -21,7 +21,7 @@ defmodule ExOpentokTest do first_item = @list["items"] |> Enum.at(0) first_item["id"] - |> Archive.find() + |> Archive.find!() |> Helper.same_map?(Helper.archive_keys()) end @@ -31,7 +31,7 @@ defmodule ExOpentokTest do # Client.http_request() test "should get 200 when request get" do - response = + {:ok, response} = Client.http_request( "https://api.opentok.com/v2/project/#{@api_key}/archive?offset=0&count=1000", :get @@ -40,16 +40,16 @@ defmodule ExOpentokTest do assert response.status == 200 end - # Client.handle_response() + # Client.handle_response!() test "should handle response 200" do response = Mock.http_response_archive_list() - assert Client.handle_response(response) == Mock.archive_list() + assert Client.handle_response!(response) == Mock.archive_list() end - # Client.handle_response() + # Client.handle_response!() test "should raise if no response 200" do assert_raise RuntimeError, fn -> - Mock.http_response_error() |> Client.handle_response() + Mock.http_response_error() |> Client.handle_response!() end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 587b9df..6bbe187 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -34,61 +34,64 @@ end defmodule ExOpentokTest.Mock do def session do - %{ - :api_key => "01234567", - :token => - "T1==cGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3NcGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3NcGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3NcGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3N==", - "create_dt" => "Tue Apr 11 08:56:40 PDT 2017", - "ice_credential_expiration" => 86100, - "ice_server" => nil, - "ice_servers" => nil, - "media_server_hostname" => nil, - "media_server_url" => "", - "messaging_server_url" => nil, - "messaging_url" => nil, - "partner_id" => "01234567", - "project_id" => "01234567", - "properties" => nil, - "session_id" => "1_MX40MX40NTgxMMX40NTgxMMX40NTgxMMX40NTgxMMX40NTgxMMX40NTg", - "session_status" => nil, - "status_invalid" => nil, - "symphony_address" => nil - } + {:ok, + %{ + :api_key => "01234567", + :token => + "T1==cGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3NcGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3NcGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3NcGFydG5lcl9pZD00NTgxMTExMiZzaWc9OUVBOTIyQjlFQzM0NUIxNkI3N==", + "create_dt" => "Tue Apr 11 08:56:40 PDT 2017", + "ice_credential_expiration" => 86100, + "ice_server" => nil, + "ice_servers" => nil, + "media_server_hostname" => nil, + "media_server_url" => "", + "messaging_server_url" => nil, + "messaging_url" => nil, + "partner_id" => "01234567", + "project_id" => "01234567", + "properties" => nil, + "session_id" => "1_MX40MX40NTgxMMX40NTgxMMX40NTgxMMX40NTgxMMX40NTgxMMX40NTg", + "session_status" => nil, + "status_invalid" => nil, + "symphony_address" => nil + }} end def http_response_archive_list do - %{ - body: - "{\"count\":1,\"items\":[{\"id\":\"5c48fb13-f27a-465d-94a7-e581d8ed49f9\",\"status\":\"expired\",\"name\":\"Ruby Archiving Sample App\",\"reason\":\"user initiated\",\"sessionId\":\"1_MX40NTgxMTExMn5-MTQ5MTY4MTU4NzU4Nn5VVU8yY1FsdThVVUU5UUVzd1VkTEh1SDJ-fg\",\"projectId\":45811112,\"createdAt\":1491682306000,\"size\":1742751,\"duration\":21,\"outputMode\":\"composed\",\"hasAudio\":true,\"hasVideo\":true,\"sha256sum\":\"97+WlyjwtOvrrNr2zG8NBTvZgqpSq5PGdgonBOTK7Fw=\",\"password\":\"\",\"updatedAt\":1491943883000,\"url\":null,\"partnerId\":45811112}]}", - headers: %{ - hdrs: %{ - "connection" => "keep-alive", - "content-type" => "application/json", - "date" => "Wed, 12 Apr 2017 08:25:59 GMT", - "server" => "nginx", - "strict-transport-security" => "max-age=31536000; includeSubdomains", - "transfer-encoding" => "chunked" - } - }, - status: 200 - } + {:ok, + %{ + body: + "{\"count\":1,\"items\":[{\"id\":\"5c48fb13-f27a-465d-94a7-e581d8ed49f9\",\"status\":\"expired\",\"name\":\"Ruby Archiving Sample App\",\"reason\":\"user initiated\",\"sessionId\":\"1_MX40NTgxMTExMn5-MTQ5MTY4MTU4NzU4Nn5VVU8yY1FsdThVVUU5UUVzd1VkTEh1SDJ-fg\",\"projectId\":45811112,\"createdAt\":1491682306000,\"size\":1742751,\"duration\":21,\"outputMode\":\"composed\",\"hasAudio\":true,\"hasVideo\":true,\"sha256sum\":\"97+WlyjwtOvrrNr2zG8NBTvZgqpSq5PGdgonBOTK7Fw=\",\"password\":\"\",\"updatedAt\":1491943883000,\"url\":null,\"partnerId\":45811112}]}", + headers: %{ + hdrs: %{ + "connection" => "keep-alive", + "content-type" => "application/json", + "date" => "Wed, 12 Apr 2017 08:25:59 GMT", + "server" => "nginx", + "strict-transport-security" => "max-age=31536000; includeSubdomains", + "transfer-encoding" => "chunked" + } + }, + status: 200 + }} end def http_response_error do - %{ - body: "{}", - headers: %{ - hdrs: %{ - "connection" => "keep-alive", - "content-type" => "application/json", - "date" => "Wed, 12 Apr 2017 08:25:59 GMT", - "server" => "nginx", - "strict-transport-security" => "max-age=31536000; includeSubdomains", - "transfer-encoding" => "chunked" - } - }, - status: 400 - } + {:ok, + %{ + body: "{}", + headers: %{ + hdrs: %{ + "connection" => "keep-alive", + "content-type" => "application/json", + "date" => "Wed, 12 Apr 2017 08:25:59 GMT", + "server" => "nginx", + "strict-transport-security" => "max-age=31536000; includeSubdomains", + "transfer-encoding" => "chunked" + } + }, + status: 400 + }} end def archive_list do