Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise Error: Add Faraday::RequestTimeoutError #1513

Merged
merged 3 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,12 @@ $ bundle exec jekyll serve
# The site will now be reachable at http://127.0.0.1:4000/faraday/
```

On newer Ruby versions (>= 3.0) `eventmachine` needs a little help to find OpenSSL 1.1 to get compiled (see <https://github.com/eventmachine/eventmachine/issues/936>). If you're using homebrew on macOS, you can do this:

```bash
brew install openssl@1.1
bundle config build.eventmachine --with-openssl-dir=$(brew --prefix openssl@1.1)
bundle install
```

Comment on lines +21 to +28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out @tisba.
Faraday doesn't bundle an eventmachine adapter since v2.0, so people should not have this issue unless they're using https://github.com/lostisland/faraday-em_http or https://github.com/lostisland/faraday-em_synchrony.

I'd suggest moving this suggestion in those gems because eventmachine is not a dependency in this gem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was about the "get Jekyll running, to view docs"? I could be wrong!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @olleolleolle said. This is about getting the docs running locally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really?! Well that's a surprise 😄!
In that case, thanks for the addition 🙏 !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Gemfile:
  github-pages was resolved to 228, which depends on
    jekyll-avatar was resolved to 0.7.0, which depends on
      jekyll was resolved to 3.9.3, which depends on
        em-websocket was resolved to 0.5.3, which depends on
          eventmachine

yup, coming form the docs/Gemfile, good catch!

[website]: https://lostisland.github.io/faraday
15 changes: 8 additions & 7 deletions docs/middleware/response/raise_error.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ Specific exceptions are raised based on the HTTP Status code, according to the l
An HTTP status in the 400-499 range typically represents an error
by the client. They raise error classes inheriting from `Faraday::ClientError`.

* 400 => `Faraday::BadRequestError`
* 401 => `Faraday::UnauthorizedError`
* 403 => `Faraday::ForbiddenError`
* 404 => `Faraday::ResourceNotFound`
* 407 => `Faraday::ProxyAuthError`
* 409 => `Faraday::ConflictError`
* 422 => `Faraday::UnprocessableEntityError`
* [400](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/400) => `Faraday::BadRequestError`
* [401](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/401) => `Faraday::UnauthorizedError`
* [403](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/403) => `Faraday::ForbiddenError`
* [404](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404) => `Faraday::ResourceNotFound`
* [407](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/407) => `Faraday::ProxyAuthError`
* [408](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408) => `Faraday::RequestTimeoutError`
* [409](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409) => `Faraday::ConflictError`
* [422](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422) => `Faraday::UnprocessableEntityError`
* 4xx => `Faraday::ClientError`

An HTTP status in the 500-599 range represents a server error, and raises a
Expand Down
4 changes: 4 additions & 0 deletions lib/faraday/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ class ResourceNotFound < ClientError
class ProxyAuthError < ClientError
end

# Raised by Faraday::Response::RaiseError in case of a 408 response.
class RequestTimeoutError < ClientError
end

# Raised by Faraday::Response::RaiseError in case of a 409 response.
class ConflictError < ClientError
end
Expand Down
2 changes: 2 additions & 0 deletions lib/faraday/response/raise_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ def on_complete(env)
# mimic the behavior that we get with proxy requests with HTTPS
msg = %(407 "Proxy Authentication Required")
raise Faraday::ProxyAuthError.new(msg, response_values(env))
when 408
raise Faraday::RequestTimeoutError, response_values(env)
when 409
raise Faraday::ConflictError, response_values(env)
when 422
Expand Down
12 changes: 12 additions & 0 deletions spec/faraday/response/raise_error_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
stub.get('forbidden') { [403, { 'X-Reason' => 'because' }, 'keep looking'] }
stub.get('not-found') { [404, { 'X-Reason' => 'because' }, 'keep looking'] }
stub.get('proxy-error') { [407, { 'X-Reason' => 'because' }, 'keep looking'] }
stub.get('request-timeout') { [408, { 'X-Reason' => 'because' }, 'keep looking'] }
stub.get('conflict') { [409, { 'X-Reason' => 'because' }, 'keep looking'] }
stub.get('unprocessable-entity') { [422, { 'X-Reason' => 'because' }, 'keep looking'] }
stub.get('4xx') { [499, { 'X-Reason' => 'because' }, 'keep looking'] }
Expand Down Expand Up @@ -79,6 +80,17 @@
end
end

it 'raises Faraday::RequestTimeoutError for 408 responses' do
expect { conn.get('request-timeout') }.to raise_error(Faraday::RequestTimeoutError) do |ex|
expect(ex.message).to eq('the server responded with status 408')
expect(ex.response[:headers]['X-Reason']).to eq('because')
expect(ex.response[:status]).to eq(408)
expect(ex.response_status).to eq(408)
expect(ex.response_body).to eq('keep looking')
expect(ex.response_headers['X-Reason']).to eq('because')
end
end

it 'raises Faraday::ConflictError for 409 responses' do
expect { conn.get('conflict') }.to raise_error(Faraday::ConflictError) do |ex|
expect(ex.message).to eq('the server responded with status 409')
Expand Down