riffed
riffed copied to clipboard
pinterest
Add exceptions
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.
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.
Good job! I had found the generated struct_names() not include exception names, so I add a PR(https://github.com/apache/thrift/pull/975) to thrift compiler to generate exception_names() meta info.
@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.
Sorry for not getting back sooner. I'll take a look at this asap, but at first glance it looks great 👍
Ah, sorry for not getting back. I'm having trouble building thrift 0.10, havn't had time to debug the issue yet.
@tbug apparently, so is apache ;)
To say we're disappointed with the speed of thrift releases is an understatement.
@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?
@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 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.