exceptions


Adam Wiggins <adam@...>
 

One of my favorite features of restclient is that it throws an
exception when you get a non-200 status code. However, this whole
area is something that is a little weak in implementation, design, and
docs.

=== Overview

For those that don't know, non-2xx status codes in restclient works as
follows. In most cases you can run a RestClient get/put/post/delete -
if the call fails it will abort, just like you would expect with (for
example) a file operation that failed. If it succeeds your code
continues to execute.

In the cases where you care about trapping specific error responses,
you can do so. For example, you might want to ignore a 404 error on
DELETE (meaning the resource was already deleted):

begin
RestClient.delete "http://somewhere/resource"
rescue RestClient::ResourceNotFound
# already deleted
end

You may also want to do something specific with the response. For
example, printing it out:

begin
RestClient.get "http://somewhere"
rescue RestClient::RequestFailed => e
puts "The request failed with HTTP status code #{e.response.code}"
puts "The body was:"
puts e.response.body
end

=== Implementation

The implementation is lacking in that the value on e.reponse is the
Net::HTTP response, not a RestClient::Response. This is because it
predates the creation of RestClient::Response. Also,
RestClient::Reponse is a little wonky right now - there's this weird
mixin thing, in general it needs some cleanup and normalization.

Although we've never documented the error handling, LOTS of things
rely on it. So we should be careful about changing the behavior of
e.response. Most importantly is that e.response.code and
e.response.body continue to work.

This would also fix the annoying problem where e.response.body needs
to be manually unzipped. RestClient asks for compressed responses and
uncompresses them automatically, but e.response.body is the raw
net::http response so you get gobblygook when you try to print out a
response from a webserver that uses gzip.

=== Design

I think the overall approach is sound, but there are some small design
issues that bother me. One is that there are specific exceptions for
certain http codes (e.g., ResourceNotFound for 404); and then a
generic catch-all (RestClient::RequestFailed) for everything else.
This results in people writing code like this:

begin
RestClient.post("http://somewhere/resource", 'post body')
rescue RestClient::RequestFailed => e
raise unless e.http_code == 422
raise(BadRecord, e.response.body)
end

Here we want to trap the specific response of 422 (which Rails uses
for failed activerecord validations) and follow some custom code path
to handle that case. For other errors we want to abort normally.
It's a shame this can't be more specific, but I don't see any easy way
around this, other than trying to create an exception for every single
HTTP code. (I think Net::HTTP does this, actually.) Even then, it's
within the HTTP spec to use custom codes - the 422 that Rails uses is
not part of the base spec:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

(I believe they borrowed it from webdav.)

My other design niggle is that it's hard to use this from the
restclient shell. I generally want to see the response body in the
shell, but you have to type a multiline command in order to do so.
One option would be to make the shell behave differently - it always
shows the output body. Another would be to add a set of
get/put/post/delete methods that behave differently and can be called
anywhere. For example:

res = RestClient.get! 'http://example.com/does-not-exist'
puts res.code # 404
puts res # body

=== Docs

This one is easy: there's no docs for exception handling. I haven't
wanted to document it because the implementation and design have never
been locked down. Once they are, it's a matter of adding it to the
readme.


Archiloque <code@...>
 

Le 15 janv. 10 à 00:11, Adam Wiggins a écrit :

One of my favorite features of restclient is that it throws an
exception when you get a non-200 status code. However, this whole
area is something that is a little weak in implementation, design, and
docs.

=== Overview

For those that don't know, non-2xx status codes in restclient works as
follows. In most cases you can run a RestClient get/put/post/delete -
if the call fails it will abort, just like you would expect with (for
example) a file operation that failed. If it succeeds your code
continues to execute.

In the cases where you care about trapping specific error responses,
you can do so. For example, you might want to ignore a 404 error on
DELETE (meaning the resource was already deleted):

begin
RestClient.delete "http://somewhere/resource"
rescue RestClient::ResourceNotFound
# already deleted
end

You may also want to do something specific with the response. For
example, printing it out:

begin
RestClient.get "http://somewhere"
rescue RestClient::RequestFailed => e
puts "The request failed with HTTP status code #{e.response.code}"
puts "The body was:"
puts e.response.body
end

=== Implementation

The implementation is lacking in that the value on e.reponse is the
Net::HTTP response, not a RestClient::Response. This is because it
predates the creation of RestClient::Response. Also,
RestClient::Reponse is a little wonky right now - there's this weird
mixin thing, in general it needs some cleanup and normalization.

Although we've never documented the error handling, LOTS of things
rely on it. So we should be careful about changing the behavior of
e.response. Most importantly is that e.response.code and
e.response.body continue to work.

This would also fix the annoying problem where e.response.body needs
to be manually unzipped. RestClient asks for compressed responses and
uncompresses them automatically, but e.response.body is the raw
net::http response so you get gobblygook when you try to print out a
response from a webserver that uses gzip.
My proposed solution : return a RestClient::Response and add
compatibility by using a missing_method wired on the underlying
Net::HTTP response with a warning

Alos, can you tell me which response part needs cleaning ?


=== Design

I think the overall approach is sound, but there are some small design
issues that bother me. One is that there are specific exceptions for
certain http codes (e.g., ResourceNotFound for 404); and then a
generic catch-all (RestClient::RequestFailed) for everything else.
This results in people writing code like this:

begin
RestClient.post("http://somewhere/resource", 'post body')
rescue RestClient::RequestFailed => e
raise unless e.http_code == 422
raise(BadRecord, e.response.body)
end

Here we want to trap the specific response of 422 (which Rails uses
for failed activerecord validations) and follow some custom code path
to handle that case. For other errors we want to abort normally.
It's a shame this can't be more specific, but I don't see any easy way
around this, other than trying to create an exception for every single
HTTP code. (I think Net::HTTP does this, actually.) Even then, it's
within the HTTP spec to use custom codes - the 422 that Rails uses is
not part of the base spec:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

(I believe they borrowed it from webdav.)
I think having a specific exception for each code is cleaner, even if
some of them will probably never be used.

For specific codes: we could probably add a hook to enable the calling
code to register them but I think that for such cases the rest-client
calls are encapsulated and custom rescue code in the encapsulating
code would be cleaner. (Unless of course someone can show me a valid
case.)

My other design niggle is that it's hard to use this from the
restclient shell. I generally want to see the response body in the
shell, but you have to type a multiline command in order to do so.
One option would be to make the shell behave differently - it always
shows the output body. Another would be to add a set of
get/put/post/delete methods that behave differently and can be called
anywhere. For example:

res = RestClient.get! 'http://example.com/does-not-exist'
puts res.code # 404
puts res # body

The idea is nice but we need to find a better name because the ! don't
match the general ! usage in ruby

A.


Blake Mizerany <blake.mizerany@...>
 

As a user of RestClient for quite some time; I've found that raising
an exception
on anything other than 200 has become my least favorite feature.

In most cases, a non-200 status is not an exceptional problem. There is just
something more that needs to be done.

Let's take a 201 for example. This means my request was successful but I need
to look elsewhere for my data. An exception is the wrong thing here.

Having exceptions thrown about prevents me from making a better decision of
what needs to be done.

My proposal is, let's have our cake and eat it too.

We break the API. Follow me on this:

RestClient.get "http://sinatrarb.com/a304" # =>
<RestClient::Response#... @code=304 ...>
RestClient.get! "http://sinatrarb.com/a304" # raises exception

The main point here is that a 304 is not an exceptional problem. In
fact, most of the time,
I don't care; it's a noop. Just keep on going.

Going a step further:

RestClient.get! "http://sinatrarb.com/asdf", :ok => [200, 304] #
=> raises if anything other than :ok

This last example gives more flexibility and power to users.
Sometimes a 304 is ok. It simply means
the user should already have what they're looking for.

IMHO, anything other than a 500 isn't an exceptional problem. In
cases where it is,
let the user decide the executions fate.

-blake

On Sat, Jan 16, 2010 at 3:44 AM, Archiloque <code@...> wrote:

Le 15 janv. 10 à 00:11, Adam Wiggins a écrit :

One of my favorite features of restclient is that it throws an
exception when you get a non-200 status code.  However, this whole
area is something that is a little weak in implementation, design, and
docs.

=== Overview

For those that don't know, non-2xx status codes in restclient works as
follows.  In most cases you can run a RestClient get/put/post/delete -
if the call fails it will abort, just like you would expect with (for
example) a file operation that failed.  If it succeeds your code
continues to execute.

In the cases where you care about trapping specific error responses,
you can do so.  For example, you might want to ignore a 404 error on
DELETE (meaning the resource was already deleted):

begin
 RestClient.delete "http://somewhere/resource"
rescue RestClient::ResourceNotFound
 # already deleted
end

You may also want to do something specific with the response.  For
example, printing it out:

begin
 RestClient.get "http://somewhere"
rescue RestClient::RequestFailed => e
 puts "The request failed with HTTP status code #{e.response.code}"
 puts "The body was:"
 puts e.response.body
end

=== Implementation

The implementation is lacking in that the value on e.reponse is the
Net::HTTP response, not a RestClient::Response.  This is because it
predates the creation of RestClient::Response.  Also,
RestClient::Reponse is a little wonky right now - there's this weird
mixin thing, in general it needs some cleanup and normalization.

Although we've never documented the error handling, LOTS of things
rely on it.  So we should be careful about changing the behavior of
e.response.  Most importantly is that e.response.code and
e.response.body continue to work.

This would also fix the annoying problem where e.response.body needs
to be manually unzipped.  RestClient asks for compressed responses and
uncompresses them automatically, but e.response.body is the raw
net::http response so you get gobblygook when you try to print out a
response from a webserver that uses gzip.
My proposed solution : return a RestClient::Response and add
compatibility by using a missing_method wired on the underlying
Net::HTTP response with a warning

Alos, can you tell me which response part needs cleaning ?


=== Design

I think the overall approach is sound, but there are some small design
issues that bother me.  One is that there are specific exceptions for
certain http codes (e.g., ResourceNotFound for 404); and then a
generic catch-all (RestClient::RequestFailed) for everything else.
This results in people writing code like this:

begin
 RestClient.post("http://somewhere/resource", 'post body')
rescue RestClient::RequestFailed => e
 raise unless e.http_code == 422
 raise(BadRecord, e.response.body)
end

Here we want to trap the specific response of 422 (which Rails uses
for failed activerecord validations) and follow some custom code path
to handle that case.  For other errors we want to abort normally.
It's a shame this can't be more specific, but I don't see any easy way
around this, other than trying to create an exception for every single
HTTP code.  (I think Net::HTTP does this, actually.)  Even then, it's
within the HTTP spec to use custom codes - the 422 that Rails uses is
not part of the base spec:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

(I believe they borrowed it from webdav.)
I think having a specific exception for each code is cleaner, even if
some of them will probably never be used.

For specific codes: we could probably add a hook to enable the calling
code to register them but I think that for such cases the rest-client
calls are encapsulated and custom rescue code in the encapsulating
code would be cleaner. (Unless of course someone can show me a valid
case.)

My other design niggle is that it's hard to use this from the
restclient shell.  I generally want to see the response body in the
shell, but you have to type a multiline command in order to do so.
One option would be to make the shell behave differently - it always
shows the output body.  Another would be to add a set of
get/put/post/delete methods that behave differently and can be called
anywhere.  For example:

res = RestClient.get! 'http://example.com/does-not-exist'
puts res.code   # 404
puts res        # body

The idea is nice but we need to find a better name because the ! don't
match the general ! usage in ruby

A.
--
Blake Mizerany
blake.mizerany@...


code@...
 

As a user of RestClient for quite some time; I've found that raising
an exception
on anything other than 200 has become my least favorite feature.

In most cases, a non-200 status is not an exceptional problem. There is
just
something more that needs to be done.

Let's take a 201 for example. This means my request was successful but I
need
to look elsewhere for my data. An exception is the wrong thing here.

Having exceptions thrown about prevents me from making a better decision
of
what needs to be done.
I think Adams was not talking about non 200 but about non 2XX, like in
second paragraph

My proposal is, let's have our cake and eat it too.

We break the API. Follow me on this:

RestClient.get "http://sinatrarb.com/a304" # =>
<RestClient::Response#... @code=304 ...>
RestClient.get! "http://sinatrarb.com/a304" # raises exception

The main point here is that a 304 is not an exceptional problem. In
fact, most of the time,
I don't care; it's a noop. Just keep on going.

Going a step further:

RestClient.get! "http://sinatrarb.com/asdf", :ok => [200, 304] #
=> raises if anything other than :ok

This last example gives more flexibility and power to users.
Sometimes a 304 is ok. It simply means
the user should already have what they're looking for.

IMHO, anything other than a 500 isn't an exceptional problem. In
cases where it is,
let the user decide the executions fate.
I'm half-convinced to manage the 304 as a 2XX because it would break the
behavior's coherency for a rare case.

I like the :ok parameter because it's explicit, but I would add it to
RestClient.get method instead of RestClient.get!, as the idea behind get!
(I would still prefer a better name) is to return a response for all
codes.

So I propose

RestClient.get "http://sinatrarb.com/asdf", :ok => [200, 304]
-> RestClient::Response for 201 and 304, exeption for anything else

RestClient.get! "http://sinatrarb.com/asdf"
-> Exception in all cases

And the exceptions hold a properly-decoded RestClient::Response (with
compatibility to http response for existing code)

btw: you said "anything other than a 500 isn't an exceptional problem":
including the 4XX ?

A.


Blake Mizerany <blake.mizerany@...>
 

Correct. Anything other than a 500 is "exceptional".

I feel that 400's are "exceptional" in some cases but not all. I want
to make the decision, not me client.

That is why I, like you, am leaning towards the :ok param. I noticed
a few days ago that Excon http://github.com/geemus/excon/ has an
:expects param which I find useful but not compelling enough to move
from RestClient.

Maybe borrowing :expects from Excon could be the right answer here.

Thoughts?

On Sunday, January 17, 2010, <code@...> wrote:
As a user of RestClient for quite some time; I've found that raising
an exception
on anything other than 200 has become my least favorite feature.

In most cases, a non-200 status is not an exceptional problem.  There is
just
something more that needs to be done.

Let's take a 201 for example.  This means my request was successful but I
need
to look elsewhere for my data.  An exception is the wrong thing here.

Having exceptions thrown about prevents me from making a better decision
of
what needs to be done.
I think Adams was not talking about non 200 but about non 2XX, like in
second paragraph

My proposal is, let's have our cake and eat it too.

We break the API.  Follow me on this:

    RestClient.get "http://sinatrarb.com/a304" # =>
<RestClient::Response#... @code=304 ...>
    RestClient.get! "http://sinatrarb.com/a304" # raises exception

The main point here is that a 304 is not an exceptional problem.  In
fact, most of the time,
I don't care; it's a noop.  Just keep on going.

Going a step further:

    RestClient.get! "http://sinatrarb.com/asdf", :ok => [200, 304] #
=> raises if anything other than :ok

This last example gives more flexibility and power to users.
Sometimes a 304 is ok.  It simply means
the user should already have what they're looking for.

IMHO, anything other than a 500 isn't an exceptional problem.   In
cases where it is,
let the user decide the executions fate.
I'm half-convinced to manage the 304 as a 2XX because it would break the
behavior's coherency for a rare case.

I like the :ok parameter because it's explicit, but I would add it to
RestClient.get method instead of RestClient.get!, as the idea behind get!
(I would still prefer a better name) is to return a response for all
codes.

So I propose

RestClient.get "http://sinatrarb.com/asdf", :ok => [200, 304]
-> RestClient::Response for 201 and 304, exeption for anything else

RestClient.get! "http://sinatrarb.com/asdf"
-> Exception in all cases

And the exceptions hold a properly-decoded RestClient::Response (with
compatibility to http response for existing code)

btw: you said "anything other than a 500 isn't an exceptional problem":
including the 4XX ?

A.


--
Blake Mizerany
blake.mizerany@...


Blake Mizerany <blake.mizerany@...>
 

An additional response:

The idea behind #get! Is that it raises on everything other than 2XX
or :excepts; the opposite for #get.

On Monday, January 18, 2010, Blake Mizerany <blake.mizerany@...> wrote:
Correct. Anything other than a 500 is "exceptional".

I feel that 400's are "exceptional" in some cases but not all. I want
to make the decision, not me client.

That is why I, like you, am leaning towards the :ok param.  I noticed
a few days ago that Excon http://github.com/geemus/excon/ has an
:expects param which I find useful but not compelling enough to move
from RestClient.

Maybe borrowing :expects from Excon could be the right answer here.

Thoughts?



On Sunday, January 17, 2010,  <code@...> wrote:
As a user of RestClient for quite some time; I've found that raising
an exception
on anything other than 200 has become my least favorite feature.

In most cases, a non-200 status is not an exceptional problem.  There is
just
something more that needs to be done.

Let's take a 201 for example.  This means my request was successful but I
need
to look elsewhere for my data.  An exception is the wrong thing here.

Having exceptions thrown about prevents me from making a better decision
of
what needs to be done.
I think Adams was not talking about non 200 but about non 2XX, like in
second paragraph

My proposal is, let's have our cake and eat it too.

We break the API.  Follow me on this:

    RestClient.get "http://sinatrarb.com/a304" # =>
<RestClient::Response#... @code=304 ...>
    RestClient.get! "http://sinatrarb.com/a304" # raises exception

The main point here is that a 304 is not an exceptional problem.  In
fact, most of the time,
I don't care; it's a noop.  Just keep on going.

Going a step further:

    RestClient.get! "http://sinatrarb.com/asdf", :ok => [200, 304] #
=> raises if anything other than :ok

This last example gives more flexibility and power to users.
Sometimes a 304 is ok.  It simply means
the user should already have what they're looking for.

IMHO, anything other than a 500 isn't an exceptional problem.   In
cases where it is,
let the user decide the executions fate.
I'm half-convinced to manage the 304 as a 2XX because it would break the
behavior's coherency for a rare case.

I like the :ok parameter because it's explicit, but I would add it to
RestClient.get method instead of RestClient.get!, as the idea behind get!
(I would still prefer a better name) is to return a response for all
codes.

So I propose

RestClient.get "http://sinatrarb.com/asdf", :ok => [200, 304]
-> RestClient::Response for 201 and 304, exeption for anything else

RestClient.get! "http://sinatrarb.com/asdf"
-> Exception in all cases

And the exceptions hold a properly-decoded RestClient::Response (with
compatibility to http response for existing code)

btw: you said "anything other than a 500 isn't an exceptional problem":
including the 4XX ?

A.


--
Blake Mizerany
blake.mizerany@...
--
Blake Mizerany
blake.mizerany@...


Archiloque <code@...>
 

A proposition elaborated with Cyril Rohr :

- add a new parameter :response (a better name would be nice), the
value would be a Proc that define how to handle the response, it will
be called with the Response as parameter
- a new Response.default_behavior method (better name welcome again)
would be added, that would represent the current behavior (return the
response itself for code between 200 and 206, raise an exception for
other cases)

Some code smaple will make this cleaner

# Default behavior
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response.default_behavior }

# No exception, always the response
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response}

# Use a specific exception for a custom http code
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| (response.code == 412) ? raise MyCustomException :
response.default_behavior }

etc.


IMHO it's simpler and more powerful, what's your opinions ?


Regards

A.


will leinweber <will@...>
 

How about 

RestClient.get, :412 => :handle_412, # call that method with the response as a param
                       :409 => lambda { |response|  #do something inline }
                       # other codes you're not interested in rase errors like normal

I like this because with CouchDB, the only thing I'm really interested in is 409s

- Will Leinweber

On Thu, Jan 21, 2010 at 12:15 PM, Archiloque <code@...> wrote:
A proposition elaborated with Cyril Rohr :

- add a new parameter :response (a better name would be nice), the
value would be a Proc that define how to handle the response, it will
be called with the Response as parameter
- a new Response.default_behavior method (better name welcome again)
would be added, that would represent the current behavior (return the
response itself for code between 200 and 206, raise an exception for
other cases)

Some code smaple will make this cleaner

# Default behavior
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response.default_behavior }

# No exception, always the response
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response}

# Use a specific exception for a custom http code
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| (response.code == 412) ? raise MyCustomException :
response.default_behavior }

etc.


IMHO it's simpler and more powerful, what's your opinions ?


Regards

A.


Cyril ROHR <cyril.rohr@...>
 

Actually I was thinking about a block, not an additional option:

response = RestClient.get "...." { |response|
  case response.status
  when 412
    do_something
  when 409, ...
    do_something
  when 500..599
    raise SomeCustomExceptionIfYouWant
  else 
    response.return!
  end
}

It doesn't change the API, and allows for a more flexible usage.

Cyril


On Thu, Jan 21, 2010 at 7:15 PM, Archiloque <code@...> wrote:
A proposition elaborated with Cyril Rohr :

- add a new parameter :response (a better name would be nice), the
value would be a Proc that define how to handle the response, it will
be called with the Response as parameter
- a new Response.default_behavior method (better name welcome again)
would be added, that would represent the current behavior (return the
response itself for code between 200 and 206, raise an exception for
other cases)

Some code smaple will make this cleaner

# Default behavior
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response.default_behavior }

# No exception, always the response
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response}

# Use a specific exception for a custom http code
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| (response.code == 412) ? raise MyCustomException :
response.default_behavior }

etc.


IMHO it's simpler and more powerful, what's your opinions ?


Regards

A.


"François Beausoleil <francois@...>
 

In the example you gave, it has to be a do/end, but I like this usage a lot.

RestClient.get "" do |response|
  p response.code
end

RestClient.get(...) {|resp| p resp.code }

{} binds very thightly, and would be passed to the String/URL parameter, instead of the #get call.

Bye!
François

Le 2010-01-21 à 14:38, Cyril ROHR a écrit :

Actually I was thinking about a block, not an additional option:

response = RestClient.get "...." { |response|
  case response.status
  when 412
    do_something
  when 409, ...
    do_something
  when 500..599
    raise SomeCustomExceptionIfYouWant
  else 
    response.return!
  end
}

It doesn't change the API, and allows for a more flexible usage.

Cyril


On Thu, Jan 21, 2010 at 7:15 PM, Archiloque <code@...> wrote:
A proposition elaborated with Cyril Rohr :

- add a new parameter :response (a better name would be nice), the
value would be a Proc that define how to handle the response, it will
be called with the Response as parameter
- a new Response.default_behavior method (better name welcome again)
would be added, that would represent the current behavior (return the
response itself for code between 200 and 206, raise an exception for
other cases)

Some code smaple will make this cleaner

# Default behavior
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response.default_behavior }

# No exception, always the response
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| response}

# Use a specific exception for a custom http code
RestClient.get 'http://example.com/resource', :response => Proc.new{|
response| (response.code == 412) ? raise MyCustomException :
response.default_behavior }

etc.


IMHO it's simpler and more powerful, what's your opinions ?


Regards

A.



code@...
 

Actually I was thinking about a block, not an additional option:
It doesn't change the API, and allows for a more flexible usage.

Cyril
Aye for the block (and the return! method), it's more rubyish in my eye
(like File.open)

@Will : I find your proposal a bit too verbose, does the block-version
suit you ?

@Blake & Adam : Would this fit your needs ?

A.


will leinweber <will@...>
 

@Will : I find your proposal a bit too verbose, does the block-version
suit you ?

Yeah, the block sounds good 


Adam Wiggins <adam@...>
 

On Thu, Jan 21, 2010 at 11:27 PM, <code@...> wrote:
Actually I was thinking about a block, not an additional option:
@Blake & Adam : Would this fit your needs ?
I dig.

Adam