[
https://issues.apache.org/jira/browse/THRIFT-5333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Denys Rtveliashvili updated THRIFT-5333:
----------------------------------------
Description:
At the moment, if an exception is defined in IDL and Java code is generated for
it, the exception would be defined as extending TException like this:
{code:java}
public class MyException extends org.apache.thrift.TException implements ...
{code}
At the same time, definition of methods looks like this:
{code:java}
public int myMethod(int argument) throws MyException,
org.apache.thrift.TException {
...
}{code}
Here, TException has to be included as that is the only way to signal that RPC
call has failed for whatever reason.
Now the problem with this is that if one calls a method like that, it is
obvious that TException covers all those exceptions defined in IDL and so there
is no way to clearly see which exceptions are result of internal workings of
Thrift and which ones correspond to situations described in IDL:
{code:java}
try {
myMethod(123);
} catch (TException e) {
log.error("Problem spotted", e);
}{code}
Why not make extensions extend java.lang.Exception instead?
{code:java}
public class MyException extends java.lang.Exception implements ...{code}
This would have made it clear which exceptions are throwable by the call as
"catch (TException e)" would not catch them and explicit catching would be in
order:
{code:java}
try {
myMethod(123);
} catch (MyException e) {
log.error("MyException has been thrown");
} catch (TException e) {
log.error("Problem spotted", e);
}
{code}
was:
At the moment, if an exception is defined in IDL and Java code is generated for
it, the exception would be defined as extending TException like this:
{code:java}
public class MyException extends org.apache.thrift.TException implements ...
{code}
At the same time, definition of methods looks like this:
{code:java}
public int myMethod(int argument) throws MyException,
org.apache.thrift.TException {
...
}{code}
Here, TException has to be included as that is the only way to signal that RPC
call has failed for whatever reason.
Now the problem with this is that if one calls a method like that, it is
obvious that TException covers all those exceptions defined in IDL and so there
is no way to clearly see which exceptions are result of internal workings of
Thrift and which ones correspond to situations described in IDL:
{code:java}
try {
myMethod(123);
} catch (TException e) {
log.error("Problem spotted", e);
}{code}
Why not make extensions extend java.lang.Exception instead?
{code:java}
public class MyException extends org.apache.thrift.TException implements
...{code}
This would have made it clear which exceptions are throwable by the call as
"catch (TException e)" would not catch them and explicit catching would be in
order:
{code:java}
try {
myMethod(123);
} catch (MyException e) {
log.error("MyException has been thrown");
} catch (TException e) {
log.error("Problem spotted", e);
}
{code}
> Exceptions defined in IDL to extend Exception rather than TException
> --------------------------------------------------------------------
>
> Key: THRIFT-5333
> URL: https://issues.apache.org/jira/browse/THRIFT-5333
> Project: Thrift
> Issue Type: Improvement
> Components: Java - Compiler
> Affects Versions: 0.13.0
> Reporter: Denys Rtveliashvili
> Priority: Major
>
> At the moment, if an exception is defined in IDL and Java code is generated
> for it, the exception would be defined as extending TException like this:
> {code:java}
> public class MyException extends org.apache.thrift.TException implements ...
> {code}
> At the same time, definition of methods looks like this:
> {code:java}
> public int myMethod(int argument) throws MyException,
> org.apache.thrift.TException {
> ...
> }{code}
> Here, TException has to be included as that is the only way to signal that
> RPC call has failed for whatever reason.
> Now the problem with this is that if one calls a method like that, it is
> obvious that TException covers all those exceptions defined in IDL and so
> there is no way to clearly see which exceptions are result of internal
> workings of Thrift and which ones correspond to situations described in IDL:
> {code:java}
> try {
> myMethod(123);
> } catch (TException e) {
> log.error("Problem spotted", e);
> }{code}
>
> Why not make extensions extend java.lang.Exception instead?
> {code:java}
> public class MyException extends java.lang.Exception implements ...{code}
> This would have made it clear which exceptions are throwable by the call as
> "catch (TException e)" would not catch them and explicit catching would be in
> order:
> {code:java}
> try {
> myMethod(123);
> } catch (MyException e) {
> log.error("MyException has been thrown");
> } catch (TException e) {
> log.error("Problem spotted", e);
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
