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.

Sign up for GitHub

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

Jump to bottom

Add exceptions #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add exceptions #20

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bef5efe
WIP, exceptions integrated into structs
Apr 12, 2016
666caee
Added exceptions to client and server
Apr 12, 2016
8dc83ab
Added "proper" exception handling
Aug 30, 2016
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
48 changes: 45 additions & 3 deletions lib/riffed/client.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,50 @@ defmodule Riffed.Client do
end
end

defp build_exception_handlers(exception_type, struct_module) do
{_seq, struct_def={:struct, {thrift_struct_module, exception_name}}} = exception_type
{:struct, detailed_meta} = :erlang.apply(thrift_struct_module, :struct_info_ext, [exception_name])
params = build_arg_list(Enum.count(detailed_meta), "_")
|> List.insert_at(0, exception_name)

args = {:{}, [], params}

quote do
defp raise_exception(ex=unquote(args)) do
ex = unquote(struct_module).to_elixir(ex, unquote(struct_def))
raise ex
end
end
end

defp build_client_function(thrift_metadata, struct_module, function_name, overrides) do
function_meta = Meta.metadata_for_function(thrift_metadata, function_name)
param_meta = function_meta[:params]
exception_meta = function_meta[:exceptions]
reply_meta = function_meta[:reply] |> Riffed.Struct.to_riffed_type_spec

reply_meta = Riffed.Enumeration.get_overridden_type(function_name, :return_type, overrides, reply_meta)

arg_list = build_arg_list(length(param_meta))
{:{}, _, list_args} = build_handler_tuple_args(param_meta)
casts = build_casts(function_name, struct_module, param_meta, overrides, :to_erlang)
exception_handlers = exception_meta
|> Enum.map(&build_exception_handlers(&1, struct_module))

quote do

unquote_splicing(exception_handlers)

def unquote(function_name)(unquote_splicing(arg_list)) do
unquote_splicing(casts)

rv = GenServer.call(__MODULE__, {unquote(function_name), unquote(list_args)})
unquote(struct_module).to_elixir(rv, unquote(reply_meta))
case rv do
{:exception, exception_record} ->
raise_exception(exception_record)

success ->
unquote(struct_module).to_elixir(rv, unquote(reply_meta))
end
end

def unquote(function_name)(client_pid, unquote_splicing(arg_list))
Expand All @@ -84,8 +111,15 @@ defmodule Riffed.Client do
unquote_splicing(casts)

rv = GenServer.call(client_pid, {unquote(function_name), unquote(list_args)})
unquote(struct_module).to_elixir(rv, unquote(reply_meta))
case rv do
{:exception, exception_record} ->
raise_exception(exception_record)
success ->
unquote(struct_module).to_elixir(success, unquote(reply_meta))
end

end

end
end

Expand Down Expand Up @@ -185,6 +219,11 @@ defmodule Riffed.Client do

unquote_splicing(client_functions)

# the default no-op for functions that don't have exceptions.
defp raise_exception(e) do
e
end

def handle_call({call_name, args}, _parent, client) do
{new_client, response} = call_thrift(client, call_name, args)
{:reply, response, new_client}
Expand All @@ -209,6 +248,8 @@ defmodule Riffed.Client do
call_thrift(new_client, call_name, args, retry_count + 1)
err = {:error, _} ->
{new_client, err}
exception = {:exception, exception_record} ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found that this isn't enough to catch the exception.. The actual erlang thrift client will throw the exception, not just return it. So when working with services that go through the thrift client (maybe the tests here don't?), the GenServer will EXIT before we reach this case statement

https://github.com/apache/thrift/blob/master/lib/erl/src/thrift_client.erl#L150

        [Exception] ->
            throw({NewClient, {exception, Exception}})

I had to modify the :thrift_client.call to look something like this:

        {thrift_client, response} = try do
          :thrift_client.call(client.client, call_name, args)
        catch
          exception_response = {thrift_client, {:exception, exception}} -> exception_response
        end

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@binaryseed is this the behavior in 0.9.3 or in head? They made a bunch of changes in head (and in the forever-upcoming 0.10 release)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe so, the code I reference is in master and I'm running based of a fork of elixir-thrift that points at master, not 0.9.3

{:new_client, exception}
{:ok, rsp} ->
{new_client, rsp}
other = {other, rsp} ->
Expand Down Expand Up @@ -252,4 +293,5 @@ defmodule Riffed.Client do
end
end
end

end
12 changes: 7 additions & 5 deletions lib/riffed/macro_helpers.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
defmodule Riffed.MacroHelpers do
@moduledoc false
def build_arg_list(size) when is_integer(size) do

def build_arg_list(size, prefix \\ nil) when is_integer(size) do
case size do
0 -> []
size ->
Enum.map(1..size, fn(param_idx) ->
:"arg_#{param_idx}"
|> Macro.var(nil)
end)
Enum.map(1..size, fn(param_idx) ->
:"#{prefix}arg_#{param_idx}"
|> Macro.var(nil)
end)
end

end

def build_handler_tuple_args(param_meta) do
Expand Down
40 changes: 36 additions & 4 deletions lib/riffed/server.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,25 +110,57 @@ defmodule Riffed.Server do
[delegate_info[:module]]}, delegate_info[:name]]}, [], arg_list}
end

defp wrap_with_exception_handlers([], _struct_module, handler_call) do
handler_call
end

defp wrap_with_exception_handlers(exceptions, struct_module, handler_call) do
elixir_exceptions = exceptions
|> Enum.map(fn({_seq,{:struct, {_thrift_module, ex_name}}}) ->
Module.concat(struct_module, ex_name)
end)

quote do
try do
unquote(handler_call)
rescue e in unquote(elixir_exceptions) -> e
erl_exc = unquote(struct_module).to_erlang(e, nil)
{:exception, erl_exc}
end
end
end

defp build_handler(meta=%Meta{}, struct_module, thrift_fn_name, delegate_fn, fn_overrides) do

function_meta = Meta.metadata_for_function(meta, thrift_fn_name)
params_meta = function_meta[:params]
reply_meta = function_meta[:reply] |> Riffed.Struct.to_riffed_type_spec
exception_meta = function_meta[:exceptions]
tuple_args = build_handler_tuple_args(params_meta)
delegate_call = build_delegate_call(delegate_fn)
casts = build_casts(thrift_fn_name, struct_module, params_meta, fn_overrides, :to_elixir)
overridden_type = Riffed.Enumeration.get_overridden_type(thrift_fn_name, :return_type, fn_overrides, reply_meta)

exception_handling = wrap_with_exception_handlers(
exception_meta,
struct_module,
quote do
unquote(delegate_call)
|> unquote(struct_module).to_erlang(unquote(overridden_type))
end)

quote do
def handle_function(unquote(thrift_fn_name), unquote(tuple_args)) do
unquote_splicing(casts)
rsp = unquote(delegate_call)
|> unquote(struct_module).to_erlang(unquote(overridden_type))
rsp = unquote(exception_handling)

case rsp do
:ok -> :ok
_ -> {:reply, rsp}
:ok ->
:ok
exc = {:exception, ex} ->
exc
_ ->
{:reply, rsp}
end
end
end
Expand Down
55 changes: 51 additions & 4 deletions lib/riffed/struct.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ defmodule Riffed.Struct do

defmodule StructData do
@moduledoc false
defstruct struct_modules: [], tuple_converters: [], struct_converters: []
defstruct struct_modules: [], tuple_converters: [], struct_converters: [], exception_modules: []

def append(data=%StructData{}, struct_module, tuple_stanza, struct_function) do
%StructData{struct_modules: [struct_module | data.struct_modules],
Expand Down Expand Up @@ -172,11 +172,44 @@ defmodule Riffed.Struct do
defstruct unquote(struct_args)

def new(opts \\ unquote(struct_args)) do
Enum.reduce(opts, %unquote(fq_module_name){}, fn({k, v}, s) -> Map.put(s, k, v) end)
opts
|> Enum.reduce(%unquote(fq_module_name){}, fn({k, v}, s) -> Map.put(s, k, v) end)
end
end
end
StructData.append(struct_data, struct_module, tuple_to_elixir, struct_to_erlang)

struct_data
|> StructData.append(struct_module, tuple_to_elixir, struct_to_erlang)
end

def build_exception_and_conversion_function(struct_data=%StructData{}, root_module, container_module, exception_module_name, thrift_module) do

{:struct, meta} = :erlang.apply(thrift_module, :struct_info_ext, [exception_module_name])
exception_args = build_struct_args(meta)
fq_module_name = Module.concat([container_module, exception_module_name])
record_name = downcase_first(exception_module_name)
record_file = "src/#{thrift_module}.hrl"
tuple_to_elixir = build_tuple_to_elixir(thrift_module, root_module, fq_module_name, meta, exception_module_name)
struct_to_erlang = build_struct_to_erlang(root_module, fq_module_name, meta, exception_module_name, record_name)

exception_module = quote do
defmodule unquote(fq_module_name) do
require Record

Record.defrecord(unquote(record_name),
Record.extract(unquote(exception_module_name), from: unquote(record_file)))

defexception unquote(exception_args)

def new(opts \\ unquote(exception_args)) do
opts
|> Enum.reduce(%unquote(fq_module_name){}, fn({k, v}, s) -> Map.put(s, k, v) end)
end
end
end

struct_data
|> StructData.append(exception_module, tuple_to_elixir, struct_to_erlang)
end

@doc false
Expand Down Expand Up @@ -279,6 +312,14 @@ defmodule Riffed.Struct do
end
end

def partition_structs_and_exceptions(thrift_module, module_names) do
struct_names = :erlang.apply(thrift_module, :struct_names, [])
|> Enum.into(MapSet.new)

module_names
|> Enum.partition(&MapSet.member?(struct_names, &1))
end

defmacro __before_compile__(env) do
options = Module.get_attribute(env.module, :thrift_options)
build_cast_to_erlang = Module.get_attribute(env.module, :build_cast_to_erlang)
Expand All @@ -288,17 +329,23 @@ defmodule Riffed.Struct do
|> Enum.reduce(
%StructData{},
fn({thrift_module, struct_names}, data) ->
{structs, exceptions} = partition_structs_and_exceptions(thrift_module,
struct_names)
curr_module = env.module
dest_module = case Keyword.get(module_mapping, thrift_module, env.module) do
^curr_module ->
curr_module
override_module ->
Module.concat([env.module, override_module])
end
Enum.reduce(struct_names, data,
data = structs
|> Enum.reduce(data,
fn(struct_name, data) ->
build_struct_and_conversion_function(data, env.module, dest_module, struct_name, thrift_module)
end)
exceptions
|> Enum.reduce(data, &(build_exception_and_conversion_function(&2, env.module, dest_module, &1, thrift_module)))

end)

callbacks = Riffed.Callbacks.build(env.module)
Expand Down
18 changes: 17 additions & 1 deletion test/riffed/client_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ defmodule ClientTest do
:getUserStates,
:getAllStates,
:getUsers,
:functionWithoutNumberedArgs
:functionWithoutNumberedArgs,
:callAndBlowUp
]

defenum ActivityState do
Expand Down Expand Up @@ -92,6 +93,13 @@ defmodule ClientTest do
end
end

def error_with(error) do
fn(client, fn_name, args) ->
EchoServer.set_args({fn_name, args})
{client, {:exception, error}}
end
end

test "it should convert nested structs into erlang" do
converted = Models.to_erlang(config_request_struct, {:struct, {:models, :ConfigRequest}})
assert {:ConfigRequest, "foo/bar", 32, {:User, "Foobie", "Barson", 1}} == converted
Expand Down Expand Up @@ -216,4 +224,12 @@ defmodule ClientTest do
assert response == 1234
assert {:functionWithoutNumberedArgs, [user_tuple, 23]} == last_call
end

test_with_mock "it should throw exceptions from the thrift server", :thrift_client, [call: error_with({:ServerException, "Server died", 500})] do

assert_raise Models.ServerException, fn ->
Client.callAndBlowUp("foo", "bar")
end
end

end
30 changes: 29 additions & 1 deletion test/riffed/server_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ defmodule ServerTest do
getMapResponse: &ServerTest.FakeHandler.get_map_response/0,
getListResponse: &ServerTest.FakeHandler.get_list_response/0,
getSetResponse: &ServerTest.FakeHandler.get_set_response/0,
getUserBoardResponse: &ServerTest.FakeHandler.get_user_board_response/0
getUserBoardResponse: &ServerTest.FakeHandler.get_user_board_response/0,
callAndBlowUp: &ServerTest.FakeHandler.call_and_blow_up/2,
],

server: {:thrift_socket_server,
Expand Down Expand Up @@ -260,6 +261,19 @@ defmodule ServerTest do
title: "Sweet stuff",
pinIds: [6, 9, 32]))
end

def call_and_blow_up(message, "usage") do
raise Data.UsageException.new(message: message, code: 291)
end

def call_and_blow_up(_, "unhandled") do
raise "oh noes!"
end

def call_and_blow_up(message, _) do
raise Data.ServerException.new(message: message, code: 392)
end

end

setup do
Expand Down Expand Up @@ -464,4 +478,18 @@ defmodule ServerTest do
Server.handle_function(:getUserBoardResponse, {})
end

test "It should properly encode and handle exceptions thrown by their handlers" do
{:exception, e} = Server.handle_function(:callAndBlowUp, {"hey there", "server"})
assert {:ServerException, "hey there", 392} == e

{:exception, e2} = Server.handle_function(:callAndBlowUp, {"usage!", "usage"})
assert {:UsageException, "usage!", 291} == e2
end

test "It should not handle exceptions not defined in thrift" do
assert_raise RuntimeError, fn ->
Server.handle_function(:callAndBlowUp, {nil, "unhandled"})
end

end
end
Loading