Skip to content

Commit

Permalink
Repurpose Down::NotFound to be raised on 404
Browse files Browse the repository at this point in the history
We've initially made Down::NotFound a catch-all to have easier
integration in Shrine, as there we wanted to convert any Down exceptions
into validation errors. However, Down::NotFound being a subclass to
almost every exception this is neither intuitive nor useful.

In fact, in Shrine 3.0 we need a Down exception that means "404" for
remote storages to be able to detect missing files and raise
Shrine::FileNotFound. So, we repurpose Down::NotFound to mean "404" and
be a descendant of Down::ClientError.

This is a breaking change, so we'll be bumping the major version.
  • Loading branch information
janko committed Sep 26, 2019
1 parent fd6b7bf commit 68754ed
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 21 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

* Drop support for `HTTP::Client` argument in `Down::HTTP.new` (@janko)

* Repurpose `Down::NotFound` to be raised on `404 Not Found` response (@janko)

## 4.8.1 (2019-05-01)

* Make `ChunkedIO#read`/`#readpartial` with length always return strings in binary encoding (@janko)
Expand Down
22 changes: 11 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ remote_file.data[:headers] #=> { ... }
remote_file.data[:response] # returns the response object
```

Note that `Down::NotFound` error will automatically be raised if response
status was 4xx or 5xx.
Note that a `Down::ResponseError` exception will automatically be raised if
response status was 4xx or 5xx.

### Down::ChunkedIO

Expand Down Expand Up @@ -210,15 +210,15 @@ the `Down::Error` subclasses. This is Down's exception hierarchy:

* `Down::Error`
* `Down::TooLarge`
* `Down::NotFound`
* `Down::InvalidUrl`
* `Down::TooManyRedirects`
* `Down::ResponseError`
* `Down::ClientError`
* `Down::ServerError`
* `Down::ConnectionError`
* `Down::TimeoutError`
* `Down::SSLError`
* `Down::InvalidUrl`
* `Down::TooManyRedirects`
* `Down::ResponseError`
* `Down::ClientError`
* `Down::NotFound`
* `Down::ServerError`
* `Down::ConnectionError`
* `Down::TimeoutError`
* `Down::SSLError`

## Backends

Expand Down
16 changes: 8 additions & 8 deletions lib/down/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,14 @@ class Error < StandardError; end
# raised when the file is larger than the specified maximum size
class TooLarge < Error; end

# raised when the file failed to be retrieved for whatever reason
class NotFound < Error; end

# raised when the given URL couldn't be parsed
class InvalidUrl < NotFound; end
class InvalidUrl < Error; end

# raised when the number of redirects was larger than the specified maximum
class TooManyRedirects < NotFound; end
class TooManyRedirects < Error; end

# raised when response returned 4xx or 5xx response
class ResponseError < NotFound
class ResponseError < Error
attr_reader :response

def initialize(message, response: nil)
Expand All @@ -29,15 +26,18 @@ def initialize(message, response: nil)
# raised when response returned 4xx response
class ClientError < ResponseError; end

# raised when response returned 404 response
class NotFound < ClientError; end

# raised when response returned 5xx response
class ServerError < ResponseError; end

# raised when there was an error connecting to the server
class ConnectionError < NotFound; end
class ConnectionError < Error; end

# raised when connecting to the server too longer than the specified timeout
class TimeoutError < ConnectionError; end

# raised when an SSL error was raised
class SSLError < NotFound; end
class SSLError < Error; end
end
1 change: 1 addition & 0 deletions lib/down/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ def response_error!(response)
args = [response.status.to_s, response: response]

case response.code
when 404 then raise Down::NotFound.new(*args)
when 400..499 then raise Down::ClientError.new(*args)
when 500..599 then raise Down::ServerError.new(*args)
else raise Down::ResponseError.new(*args)
Expand Down
1 change: 1 addition & 0 deletions lib/down/net_http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ def response_error!(response)
args = ["#{code} #{message}", response: response]

case response.code.to_i
when 404 then raise Down::NotFound.new(*args)
when 400..499 then raise Down::ClientError.new(*args)
when 500..599 then raise Down::ServerError.new(*args)
else raise Down::ResponseError.new(*args)
Expand Down
6 changes: 5 additions & 1 deletion test/http_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,14 @@
end

it "raises on HTTP error responses" do
error = assert_raises(Down::ClientError) { Down::Http.open("#{$httpbin}/status/404") }
error = assert_raises(Down::NotFound) { Down::Http.open("#{$httpbin}/status/404") }
assert_equal "404 Not Found", error.message
assert_instance_of HTTP::Response, error.response

error = assert_raises(Down::ClientError) { Down::Http.open("#{$httpbin}/status/403") }
assert_equal "403 Forbidden", error.message
assert_instance_of HTTP::Response, error.response

error = assert_raises(Down::ServerError) { Down::Http.open("#{$httpbin}/status/500") }
assert_equal "500 Internal Server Error", error.message
assert_instance_of HTTP::Response, error.response
Expand Down
6 changes: 5 additions & 1 deletion test/net_http_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,14 @@
end

it "raises on HTTP error responses" do
error = assert_raises(Down::ClientError) { Down::NetHttp.download("#{$httpbin}/status/404") }
error = assert_raises(Down::NotFound) { Down::NetHttp.download("#{$httpbin}/status/404") }
assert_equal "404 Not Found", error.message
assert_kind_of Net::HTTPResponse, error.response

error = assert_raises(Down::ClientError) { Down::NetHttp.download("#{$httpbin}/status/403") }
assert_equal "403 Forbidden", error.message
assert_kind_of Net::HTTPResponse, error.response

error = assert_raises(Down::ServerError) { Down::NetHttp.download("#{$httpbin}/status/500") }
assert_equal "500 Internal Server Error", error.message
assert_kind_of Net::HTTPResponse, error.response
Expand Down

0 comments on commit 68754ed

Please sign in to comment.