Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Improve some the error messages #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/cassette/client/generate_st.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ defmodule Cassette.Client.GenerateSt do

alias Cassette.Config
alias Cassette.Client
alias HTTPoison.Error
alias HTTPoison.Response

@type response ::
{:error, :bad_tgt}
Expand All @@ -26,15 +28,18 @@ defmodule Cassette.Client.GenerateSt do
headers = []

case post(url, params, headers, options) do
{:ok, %HTTPoison.Response{status_code: 404}} ->
{:ok, %Response{status_code: 404}} ->
{:error, :bad_tgt}

{:ok, %HTTPoison.Response{status_code: 200, body: body}} ->
{:ok, %Response{status_code: 200, body: body}} ->
{:ok, body}

{:ok, %HTTPoison.Response{status_code: status_code, body: body}} ->
{:ok, %Response{status_code: status_code, body: body}} ->
{:fail, status_code, body}

{:error, %Error{reason: reason}} ->
{:fail, reason}

_ ->
{:fail, :unknown}
end
Expand Down
7 changes: 5 additions & 2 deletions lib/cassette/client/generate_tgt.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ defmodule Cassette.Client.GenerateTgt do

alias Cassette.Config
alias Cassette.Client
alias HTTPoison.Error
alias HTTPoison.Response

@type response ::
{:error, :bad_credentials}
| {:ok, String.t()}
| {:fail, pos_integer()}
| {:fail, :unknown}
| {:fail, term()}

@spec process_headers([{String.t(), String.t()}]) :: %{String.t() => String.t()}
defp process_headers(headers) do
Expand Down Expand Up @@ -42,6 +42,9 @@ defmodule Cassette.Client.GenerateTgt do
{:ok, %Response{status_code: status_code}} ->
{:fail, status_code}

{:error, %Error{reason: reason}} when is_atom(reason) ->
{:fail, reason}

_ ->
{:fail, :unknown}
end
Expand Down
4 changes: 3 additions & 1 deletion lib/cassette/client/validate_ticket.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ defmodule Cassette.Client.ValidateTicket do

alias Cassette.Config
alias Cassette.Client
alias HTTPoison.Error
alias HTTPoison.Response

@type response :: {:ok, String.t()} | {:fail, :unknown}
@type response :: {:ok, String.t()} | {:fail, term()}

@doc """
Do request to cas service to validate a service ticket
Expand All @@ -22,6 +23,7 @@ defmodule Cassette.Client.ValidateTicket do

case get(url, headers, options) do
{:ok, %Response{status_code: 200, body: body}} -> {:ok, body}
{:error, %Error{reason: reason}} when is_atom(reason) -> {:fail, reason}
_ -> {:fail, :unknown}
end
end
Expand Down
8 changes: 7 additions & 1 deletion lib/cassette/server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ defmodule Cassette.Server do
GenServer.start_link(__MODULE__, {:ok, config})
end

version(">= 1.5.0") do
version ">= 1.5.0" do
def child_spec([name, config = %Config{}]) do
defaults = %{id: name, start: {__MODULE__, :start_link, [name, config]}}
Supervisor.child_spec(defaults, [])
Expand Down Expand Up @@ -178,6 +178,9 @@ defmodule Cassette.Server do
{:fail, :unknown} ->
{:reply, {:error, "Failed for unknown reason"}, state}

{:fail, reason} when is_atom(reason) ->
{:reply, {:error, "Failed because #{reason}"}, state}

{:fail, status_code} ->
{:reply, {:error, "Failed with status #{status_code}"}, state}
end
Expand Down Expand Up @@ -236,6 +239,9 @@ defmodule Cassette.Server do

{:fail, :unknown} ->
{:error, "Failed for unknown reason"}

{:fail, reason} ->
{:error, "Failed because #{reason}"}
end
end

Expand Down
4 changes: 2 additions & 2 deletions test/cassette/client/generate_st_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ defmodule Cassette.Client.GenerateStTest do
assert {:fail, 418, "I. am. a. freaking. teapot."} = Cassette.Client.GenerateSt.perform(config, tgt, service)
end

test "perform returns {:fail, :unknown} then http fails", %{bypass: bypass, config: config, tgt: tgt, service: service} do
test "perform returns {:fail, reason} then http fails with an atom", %{bypass: bypass, config: config, tgt: tgt, service: service} do
Bypass.down(bypass)

assert {:fail, :unknown} = Cassette.Client.GenerateSt.perform(config, tgt, service)
assert {:fail, :econnrefused} = Cassette.Client.GenerateSt.perform(config, tgt, service)
end

test "perform generates a ST", %{bypass: bypass, config: config, tgt: tgt, service: service} do
Expand Down
6 changes: 6 additions & 0 deletions test/cassette/client/generate_tgt_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ defmodule Cassette.Client.GenerateTgtTest do
assert {:error, :bad_credentials} = Cassette.Client.GenerateTgt.perform(config)
end

test "perform returns {:fail, reason} for atom error reasons", %{bypass: bypass, config: config} do
Bypass.down(bypass)

assert {:fail, :econnrefused} = Cassette.Client.GenerateTgt.perform(config)
end

test "perform returns {:fail, status_code} for other error statuses", %{bypass: bypass, config: config} do
Bypass.expect bypass, fn conn ->
conn |> Plug.Conn.resp(404, "not found")
Expand Down
12 changes: 6 additions & 6 deletions test/cassette/client/validate_ticket_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ defmodule Cassette.Client.ValidateTicketTest do
{:ok, bypass: bypass, config: config, ticket: ticket, service: service}
end

test "perform returns {:fail, reason} for atom failure reasons", %{bypass: bypass, config: config, ticket: ticket, service: service} do
Bypass.down(bypass)

assert {:fail, :econnrefused} = Cassette.Client.ValidateTicket.perform(config, ticket, service)
end

test "perform returns {:fail, :unknown} for not-200 response", %{bypass: bypass, config: config, ticket: ticket, service: service} do
Bypass.expect bypass, fn conn ->
conn |> Plug.Conn.resp(404, "not found")
Expand All @@ -19,12 +25,6 @@ defmodule Cassette.Client.ValidateTicketTest do
assert {:fail, :unknown} = Cassette.Client.ValidateTicket.perform(config, ticket, service)
end

test "perform returns {:fail, :unknown} then http fails", %{bypass: bypass, config: config, ticket: ticket, service: service} do
Bypass.down(bypass)

assert {:fail, :unknown} = Cassette.Client.ValidateTicket.perform(config, ticket, service)
end

test "perform returns the validation body", %{bypass: bypass, config: config, ticket: ticket, service: service} do
body = "<validation><result><xml /></result></validation>"

Expand Down
4 changes: 2 additions & 2 deletions test/cassette/server_integration_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ defmodule Cassette.ServerIntegrationTest do
assert {:ok, ^tgt} = Server.tgt(pid)
end

test "fails with unknown reason when cas is down",
test "fails when cas is down",
%{pid: pid, config: config} do

{:ok, fake_cas_pid} = FakeCas.Server.start_link([])
Expand All @@ -28,7 +28,7 @@ defmodule Cassette.ServerIntegrationTest do
:ok = Cassette.Server.reload(pid, config)
FakeCas.Server.stop(fake_cas_pid)

assert {:error, "Failed for unknown reason"} = Server.tgt(pid)
assert {:error, "Failed because econnrefused"} = Server.tgt(pid)
end

test "fails with {:error, _} when username/password is invalid",
Expand Down