Change declaration of ProxyBase#setRequestHeaders to throw GadgetException

This patch changes the declaration of ProxyBase#setRequestHeaders to throw a GadgetException, which makes it easier to override this method and is also consistent with ProxyBase#setResponseHeaders, which already declares to throw this kind of exception.

[gagan.goku, 2010-06-23 16:36:35]

On 2010/06/23 16:05:22, janluehe wrote:
> 

Do you want to wrap an inner exception into GadgetException?
Currently there doesn't seem to be anything in setRequestHeaders that can throw
an exception. If you have a case where some null pointer exception etc is
returned, then this change makes sense.

[janluehe, 2010-06-23 16:55:47]

We override setRequestHeaders to add some product-specific headers, and when
determining their values, checked exceptions may be thrown. Currently, we have
to log them as severe errors, but we'd really like to be able to wrap them
inside GadgetException and have ProxyBase#outputError take care of producing an
error response.


On 2010/06/23 16:36:35, gagan.goku wrote:
> On 2010/06/23 16:05:22, janluehe wrote:
> > 
> 
> Do you want to wrap an inner exception into GadgetException?
> Currently there doesn't seem to be anything in setRequestHeaders that can
throw
> an exception. If you have a case where some null pointer exception etc is
> returned, then this change makes sense.

[gagan.goku, 2010-06-23 17:31:43]

On 2010/06/23 16:55:47, janluehe wrote:
> We override setRequestHeaders to add some product-specific headers, and when
> determining their values, checked exceptions may be thrown. Currently, we have
> to log them as severe errors, but we'd really like to be able to wrap them
> inside GadgetException and have ProxyBase#outputError take care of producing
an
> error response.
> 
> 
> On 2010/06/23 16:36:35, gagan.goku wrote:
> > On 2010/06/23 16:05:22, janluehe wrote:
> > > 
> > 
> > Do you want to wrap an inner exception into GadgetException?
> > Currently there doesn't seem to be anything in setRequestHeaders that can
> throw
> > an exception. If you have a case where some null pointer exception etc is
> > returned, then this change makes sense.

Your use case sounds good to me.

But you can also do this:
  static class A {
    protected void hello() {
      throw new NullPointerException("hello");
    }
  }

  static class B extends A {
    public void hello1() throws GadgetException {
      try {
        super.hello();
      } catch (Exception e) {
        e.printStackTrace();
        throw new GadgetException(e);
      }
    }
  }

[janluehe, 2010-06-23 18:09:13]

What we are trying is different from your example. This is our use case in terms
of your example:

  static class A {
    protected void hello() {
      // do something
    }
  }

  static class B extends A {
    protected void hello() {
      super.hello();
      customHello();
    }

    private void customHello() /*throws GadgetException*/ {
      // do something that may throw GadgetException
    }
  }

We would like to be able to propagate the GadgetException from customHello(),
and have customHello() declare to throw a GadgetException, but for this to
happen, hello() also must declare to throw a GadgetException. 


On 2010/06/23 17:31:43, gagan.goku wrote:
> On 2010/06/23 16:55:47, janluehe wrote:
> > We override setRequestHeaders to add some product-specific headers, and when
> > determining their values, checked exceptions may be thrown. Currently, we
have
> > to log them as severe errors, but we'd really like to be able to wrap them
> > inside GadgetException and have ProxyBase#outputError take care of producing
> an
> > error response.
> > 
> > 
> > On 2010/06/23 16:36:35, gagan.goku wrote:
> > > On 2010/06/23 16:05:22, janluehe wrote:
> > > > 
> > > 
> > > Do you want to wrap an inner exception into GadgetException?
> > > Currently there doesn't seem to be anything in setRequestHeaders that can
> > throw
> > > an exception. If you have a case where some null pointer exception etc is
> > > returned, then this change makes sense.
> 
> Your use case sounds good to me.
> 
> But you can also do this:
>   static class A {
>     protected void hello() {
>       throw new NullPointerException("hello");
>     }
>   }
> 
>   static class B extends A {
>     public void hello1() throws GadgetException {
>       try {
>         super.hello();
>       } catch (Exception e) {
>         e.printStackTrace();
>         throw new GadgetException(e);
>       }
>     }
>   }

[fargo, 2010-06-23 18:27:24]

Thanks Gagan for commenting esp. from the accel side. @Jan, I just committed
this patch -- seems quite reasonable to me. Thanks!

On Wed, Jun 23, 2010 at 11:09 AM, <janluehe@gmail.com> wrote:

> What we are trying is different from your example. This is our use case
> in terms of your example:
>
>
>  static class A {
>    protected void hello() {
>      // do something
>
>    }
>  }
>
>  static class B extends A {
>    protected void hello() {
>      super.hello();
>      customHello();
>    }
>
>    private void customHello() /*throws GadgetException*/ {
>      // do something that may throw GadgetException
>    }
>  }
>
> We would like to be able to propagate the GadgetException from
> customHello(), and have customHello() declare to throw a
> GadgetException, but for this to happen, hello() also must declare to
> throw a GadgetException.
>
>
>
> On 2010/06/23 17:31:43, gagan.goku wrote:
>
>> On 2010/06/23 16:55:47, janluehe wrote:
>> > We override setRequestHeaders to add some product-specific headers,
>>
> and when
>
>> > determining their values, checked exceptions may be thrown.
>>
> Currently, we have
>
>> > to log them as severe errors, but we'd really like to be able to
>>
> wrap them
>
>> > inside GadgetException and have ProxyBase#outputError take care of
>>
> producing
>
>> an
>> > error response.
>> >
>> >
>> > On 2010/06/23 16:36:35, gagan.goku wrote:
>> > > On 2010/06/23 16:05:22, janluehe wrote:
>> > > >
>> > >
>> > > Do you want to wrap an inner exception into GadgetException?
>> > > Currently there doesn't seem to be anything in setRequestHeaders
>>
> that can
>
>> > throw
>> > > an exception. If you have a case where some null pointer exception
>>
> etc is
>
>> > > returned, then this change makes sense.
>>
>
>  Your use case sounds good to me.
>>
>
>  But you can also do this:
>>   static class A {
>>     protected void hello() {
>>       throw new NullPointerException("hello");
>>     }
>>   }
>>
>
>    static class B extends A {
>>     public void hello1() throws GadgetException {
>>       try {
>>         super.hello();
>>       } catch (Exception e) {
>>         e.printStackTrace();
>>         throw new GadgetException(e);
>>       }
>>     }
>>   }
>>
>
>
>
> http://codereview.appspot.com/1683050/show
>

[gagan.goku, 2010-06-23 18:32:55]

Happy to chime in.

Thanks
Gagan

-- 
The only thing missing in life is background music.
-- Gagandeep Singh


On Wed, Jun 23, 2010 at 11:57 PM, John Hjelmstad <fargo@google.com> wrote:

> Thanks Gagan for commenting esp. from the accel side. @Jan, I just
> committed
> this patch -- seems quite reasonable to me. Thanks!
>
> On Wed, Jun 23, 2010 at 11:09 AM, <janluehe@gmail.com> wrote:
>
> > What we are trying is different from your example. This is our use case
> > in terms of your example:
> >
> >
> >  static class A {
> >    protected void hello() {
> >      // do something
> >
> >    }
> >  }
> >
> >  static class B extends A {
> >    protected void hello() {
> >      super.hello();
> >      customHello();
> >    }
> >
> >    private void customHello() /*throws GadgetException*/ {
> >      // do something that may throw GadgetException
> >    }
> >  }
> >
> > We would like to be able to propagate the GadgetException from
> > customHello(), and have customHello() declare to throw a
> > GadgetException, but for this to happen, hello() also must declare to
> > throw a GadgetException.
> >
> >
> >
> > On 2010/06/23 17:31:43, gagan.goku wrote:
> >
> >> On 2010/06/23 16:55:47, janluehe wrote:
> >> > We override setRequestHeaders to add some product-specific headers,
> >>
> > and when
> >
> >> > determining their values, checked exceptions may be thrown.
> >>
> > Currently, we have
> >
> >> > to log them as severe errors, but we'd really like to be able to
> >>
> > wrap them
> >
> >> > inside GadgetException and have ProxyBase#outputError take care of
> >>
> > producing
> >
> >> an
> >> > error response.
> >> >
> >> >
> >> > On 2010/06/23 16:36:35, gagan.goku wrote:
> >> > > On 2010/06/23 16:05:22, janluehe wrote:
> >> > > >
> >> > >
> >> > > Do you want to wrap an inner exception into GadgetException?
> >> > > Currently there doesn't seem to be anything in setRequestHeaders
> >>
> > that can
> >
> >> > throw
> >> > > an exception. If you have a case where some null pointer exception
> >>
> > etc is
> >
> >> > > returned, then this change makes sense.
> >>
> >
> >  Your use case sounds good to me.
> >>
> >
> >  But you can also do this:
> >>   static class A {
> >>     protected void hello() {
> >>       throw new NullPointerException("hello");
> >>     }
> >>   }
> >>
> >
> >    static class B extends A {
> >>     public void hello1() throws GadgetException {
> >>       try {
> >>         super.hello();
> >>       } catch (Exception e) {
> >>         e.printStackTrace();
> >>         throw new GadgetException(e);
> >>       }
> >>     }
> >>   }
> >>
> >
> >
> >
> > http://codereview.appspot.com/1683050/show
> >
>