Add exceptions

[scohen]

This PR adds exceptions to the client, server and struct modules. It's dependent on thrift 0.10 though, and targets the 1.5 branch.

[scohen]

@laozhp @tbug How is this?

[jparise]

Maybe this should require Thrift 0.10 in mix.exs (via thrift_version).

The .travis.yml script will also need to an update to use Thrift 0.10 in the CI environment.

[laozhp]

Good job! I had found the generated struct_names() not include exception names, so I add a PR(apache/thrift#975) to thrift compiler to generate exception_names() meta info.

[scohen]

@jparise I can't, it's not out yet.

@laozhp Thanks for putting that fix in; I'm sad to say that I omitted exceptions when I made the original change. Once this lands, I can just ask for the exceptions rather than relying on them not being in the struct list.

[tudborg]

Sorry for not getting back sooner.
I'll take a look at this asap, but at first glance it looks great 👍

[tudborg]

Ah, sorry for not getting back.
I'm having trouble building thrift 0.10, havn't had time to debug the issue yet.

[scohen]

@tbug apparently, so is apache ;)

To say we're disappointed with the speed of thrift releases is an understatement.

[tudborg]

@scohen Ha, alright. I guess I'm happy it's not just me then 😄

I'm not really following thrift dev, so I have no idea how far we are from 0.10,
but I'd expect it to be around the corner?
Out of curiosity, which parts of this PR depends on 0.10?

[scohen]

@tbug As far as I can tell from the outside, they've tagged the release and are now doing... something.

This PR relies on deeper metadata around functions and exceptions, which were added to 0.10.

[binaryseed]

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

[scohen]

@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)

[binaryseed]

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

[scohen]

@binaryseed Sorry it's been so long, but I updated exception handling on the v1_5_0 branch. Please give it a shot.
I also made full end-to-end unit tests so that I can be sure that both the server and client handle exceptions properly.