artiya4u / google-http-java-client

Automatically exported from code.google.com/p/google-http-java-client
0 stars 0 forks source link

HttpRequest throws Exception instead of IOException [Backwards Incompatible] #137

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
External references, such as a standards document, or specification?

e.g.

http://javadoc.google-http-java-client.googlecode.com/hg/1.10.3-beta/com/google/
api/client/http/HttpRequest.html#execute()

http://javadoc.google-http-java-client.googlecode.com/hg/1.10.3-beta/com/google/
api/client/http/HttpRequestInitializer.html#initialize(com.google.api.client.htt
p.HttpRequest)

http://javadoc.google-http-java-client.googlecode.com/hg/1.10.3-beta/com/google/
api/client/http/HttpExecuteInterceptor.html#intercept(com.google.api.client.http
.HttpRequest)

Java environments (e.g. Java 6, Android 2.3, App Engine, or All)?

All

Please describe the feature requested.

Currently we only allow IOException to be thrown on HttpRequest.execute().  
This is a feature request to extend it to allow any Exception to be thrown.  
This is clearly a backwards-incompatible change.

Here's a sample scenario: suppose I have a special scenario in which I need to 
throw some custom exception called FooException inside 
HttpExecuteInterceptor.intercept(HttpRequest).  But FooException isn't an I/O 
problem.  Perhaps it is SQLException or XMLParseException or any number of 
other possible exceptions.  We don't want it to be treated as some kind of 
network connection problem and retried.  We simply need to throw it and expect 
it to be caught in the code properly.

Currently the only way is to wrap it in an IOException.  For example:

   public void intercept(HttpRequest request) throws IOException {
     try {
       // do stuff
...
     } catch (FooException exception) {
       IOException io = new IOException();
       io.initCause(exception);
       throw io;
     }
   }

then:

 protected void executeHttpRequest() {
   try {
...
   } catch (IOException e) {
     if (e.getCause() instanceof FooException) {
       FooException fe = (FooException) e.getCause();
...
     } else {
...
     }
   }
 }

In particular, the getCause() is really annoying.  Java has a very nice syntax 
with try-catch and ideally I'd like to be able to take advantage of it.  If 
instead we change intercept to throw Exception instead of IOException the code 
is much simpler:

   public void intercept(HttpRequest request) throws FooException {
       // do stuff
...
   }

then:

 protected void executeHttpRequest() {
   try {
...
   } catch (FooException fe) {
...
   } catch (IOException e) {
...
   }
 }

Original issue reported on code.google.com by yan...@google.com on 3 Jul 2012 at 9:49

GoogleCodeExporter commented 9 years ago

Original comment by rmis...@google.com on 11 Jul 2012 at 12:45

GoogleCodeExporter commented 9 years ago
Due to the many breaking references due to numerous interface changes we have 
decided to revisit this later.

Original comment by rmis...@google.com on 26 Jul 2012 at 2:57

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 7 Sep 2012 at 1:33

GoogleCodeExporter commented 9 years ago
https://codereview.appspot.com/6500102/

Original comment by yan...@google.com on 11 Sep 2012 at 12:50

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 11 Sep 2012 at 5:02

GoogleCodeExporter commented 9 years ago
Exception is too general of a type, especially as it even includes 
RuntimeException.  In the future, we will consider create a new class like 
HttpException and throw that instead.

Original comment by yan...@google.com on 23 Oct 2012 at 8:02