Skip to content

Conversation

joshuaflanagan
Copy link
Contributor

This fixes #463

This will greatly enhance the abilities of pluggable Features by
being able to reference the Request that initiated a Response. It will make it easy to solves issues like #545.

I wasn't very happy with all the changes to the specs to put a "fake" Request, but felt it was pretty important that the Request object be mandatory when creating a Response. If we left it optional, it might be too easy to create a new code path (like the auto_inflate code) that doesn't properly set the initiating Request. Open to suggestions.

@joshuaflanagan joshuaflanagan force-pushed the response_request branch 2 times, most recently from 99ac40f to 3aae264 Compare April 24, 2019 23:01
joshuaflanagan added a commit to ShippingEasy/http that referenced this pull request Apr 24, 2019
This mirrors httprb#546, which has not been
merged yet.
joshuaflanagan added a commit to ShippingEasy/webmock that referenced this pull request Apr 25, 2019
Versions of HTTP.rb that allow access to the Request from the Response (httprb/http#546) require this change to continue working.
@natesholland natesholland mentioned this pull request May 7, 2019
@scarfacedeb scarfacedeb mentioned this pull request Sep 3, 2019
joshuaflanagan added a commit to ShippingEasy/webmock that referenced this pull request Oct 5, 2019
Versions of HTTP.rb that allow access to the Request from the Response (httprb/http#546) require this change to continue working.
@joshuaflanagan
Copy link
Contributor Author

What are the outstanding issues blocking this from being merged? I’ve been running this in production for 6 months now - happy to help get it merged so others can get the improved logging ability.

@tarcieri
Copy link
Member

@joshuaflanagan if you can rebase I will review/merge

@joshuaflanagan
Copy link
Contributor Author

@tarcieri ok done. The mobile github view made it harder to tell that there were merge conflicts. All fixed now.

@@ -199,6 +199,10 @@ def inspect
"#<#{self.class}/#{@version} #{verb.to_s.upcase} #{uri}>"
end

def self.example
new(:verb => :get, :uri => "http://example.com")
end
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind moving this into the specs themselves so it isn't otherwise defined when the specs aren't running? You can still define it on HTTP::Request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm ok with removing the example from the Request definition, but don't think it is worth re-opening the class in the specs. I think that would be more confusing to readers to see the class method invoked, but not defined where you would expect to find it.

I think the specs will be more readable to just repeat new(:verb => :get, :uri => "http://example.com") where needed. I can make that change if you agree.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, that sounds good

This fixes httprb#463

This will greatly enhance the abilities of pluggable `Features` by
being able to reference the Request that initiated a Response.
During normal operation, the Response#uri will be pulled from
the Request passed into the Response. However, we still allow explicitly
setting a `:uri` option when creating a Response, for backwards compatibility.

This was motivated by rubocop/code climate complaining that `Client#perform`
was now 1-line too long. I'll defer to the project maintainers to decide
if that is a good thing.
@tarcieri tarcieri merged commit 4430932 into httprb:master Oct 21, 2019
@tarcieri
Copy link
Member

Thank you! Sorry about the long delay

ixti added a commit that referenced this pull request Oct 21, 2019
Make them closer to reality.
Related to: #546
ixti added a commit that referenced this pull request Oct 21, 2019
ixti added a commit that referenced this pull request Oct 21, 2019
* Use Request#uri in Response

Response initialize requires Request object now, making uri parsing
absolutely redundant. And as it's a breaking change anyway - we should
get rid of `:uri` option completely.

NOTE: We need to update WebMock's adapter to reflect this change prior
releasing v5.0.0

* Update Changelog with #546 changes

SEE ALSO
--------

ShippingEasy/webmock@505ad00
@jrochkind
Copy link
Contributor

jrochkind commented Nov 20, 2019

Hi, this is listed as a breaking change at https://github.com/httprb/http/blob/master/CHANGES.md (presumably for 5.0.0.pre and an upcoming 5.0.0?)

From the PR description and diff, it's not yet clear to me what the nature of the breaking change is. Can you provide more guidance?

Is the backwards incompat only if you are manually instantiating an HTTP::Response, now it takes different args? (I'd imagine manually instantiating an HTTP::Response is relatively uncommon)

Thanks!

@tarcieri
Copy link
Member

@jrochkind previously the request URI was a directly accessible member of the response object. That was removed.

However, now the original request is a directly accessible member object of the response, and the request object knows the original URI. So, it removes direct access to the request URI, but it's still available via another layer of indirection through the request.

@ixti
Copy link
Member

ixti commented Nov 21, 2019

We have #uri on response delegated to #request in fact. The breaking change is that Response object was initialized with uri option before and now it requires response instead.

This is a breaking change for integrators like WebMock for example.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 11, 2025
Changelog:
## [5.3.1] - 2025-06-09

### Changed

- Revert switch to the native llhttp on MRI, as it's not compatible with
  standalone bundles
  ([#802](httprb/http#802))


## [5.3.0] - 2025-06-09

### Added

- (backported) Add .retriable feature to Http
- (backported) Add more specific ConnectionError classes
- (backported) New feature: RaiseError

### Changed

- (backported) Drop depenency on base64
- (backported) Cache header normalization to reduce object allocation
- (backported) Use native llhttp on MRI


## [5.2.0] - 2024-02-05

### Added

- Add `Connection#finished_request?`
  ([#743](httprb/http#743))
- Add `Instrumentation#on_error`
  ([#746](httprb/http#746))
- Add `base64` dependency (suppresses warnings on Ruby 3.0)
  ([#759](httprb/http#759))
- Add `PURGE` HTTP verb
  ([#757](httprb/http#757))
- Add Ruby-3.3 support

### Changed

- **BREAKING** Process features in reverse order
  ([#766](httprb/http#766))
- **BREAKING** Downcase Content-Type charset name
  ([#753](httprb/http#753))
- **BREAKING** Make URI normalization more conservative
  ([#758](httprb/http#758))

### Fixed

- Close sockets on initialize failure
  ([#762](httprb/http#762))
- Prevent CRLF injection due to broken URL normalizer
  ([#765](httprb/http#765))

[unreleased]: httprb/http@v5.3.0...5-x-stable
[5.3.0]: httprb/http@v5.2.0...v5.3.0
[5.2.0]: httprb/http@v5.1.1...v5.2.0

## 5.1.1 (2022-12-17)

* [#731](httprb/http#731)
  Strip brackets from IPv6 addresses in `HTTP::URI`.
  ([@jeraki])

* [#722](httprb/http#722)
  Add `on_redirect` callback.
  ([@benubois])

## 5.1.0 (2022-06-17)

* Drop ruby-2.5 support.

* [#715](httprb/http#715)
  Set default encoding to UTF-8 for `application/json`.
  ([@drwl])

* [#712](httprb/http#712)
  Recognize cookies set by redirect.
  ([@tkellogg])

* [#707](httprb/http#707)
  Distinguish connection timeouts.
  ([@YuLeven])

## 5.0.4 (2021-10-07)

* [#698](httprb/http#698)
  Fix `HTTP::Timeout::Global#connect_ssl`.
  ([@tarcieri])

## 5.0.3 (2021-10-06)

* [#695](httprb/http#695)
  Revert DNS resolving feature.
  ([@PhilCoggins])

* [#694](httprb/http#694)
  Fix cookies extraction.
  ([@flosacca])

## 5.0.2 (2021-09-10)

* [#686](httprb/http#686)
  Correctly reset the parser.
  ([@bryanp])

* [#684](httprb/http#684)
  Don't set Content-Length for GET, HEAD, DELETE, or CONNECT requests without a BODY.
  ([@jyn514])

* [#679](httprb/http#679)
  Use features on redirected requests.
  ([@nomis])

* [#678](httprb/http#678)
  Restore `HTTP::Response` `:uri` option for backwards compatibility.
  ([@schwern])

* [#676](httprb/http#676)
  Update addressable because of CVE-2021-32740.
  ([@matheussilvasantos])

* [#653](httprb/http#653)
  Avoid force encodings on frozen strings.
  ([@bvicenzo])

* [#638](httprb/http#638)
  DNS failover handling.
  ([@midnight-wonderer])


## 5.0.1 (2021-06-26)

* [#670](httprb/http#670)
  Revert `Response#parse` behavior introduced in [#540].
  ([@DannyBen])

* [#669](httprb/http#669)
  Prevent bodies from being resubmitted when following unsafe redirects.
  ([@odinhb])

* [#664](httprb/http#664)
  Bump llhttp-ffi to 0.3.0.
  ([@bryanp])


## 5.0.0 (2021-05-12)

* [#656](httprb/http#656)
  Handle connection timeouts in `Features`
  ([@semenyukdmitry])

* [#651](httprb/http#651)
  Replace `http-parser` with `llhttp`
  ([@bryanp])

* [#647](httprb/http#647)
  Add support for `MKCALENDAR` HTTP verb
  ([@meanphil])

* [#632](httprb/http#632)
  Respect the SSL context's `verify_hostname` value
  ([@colemannugent])

* [#625](httprb/http#625)
  Fix inflator with empty responses
  ([@LukaszMaslej])

* [#599](httprb/http#599)
  Allow passing `HTTP::FormData::{Multipart,UrlEncoded}` object directly.
  ([@ixti])

* [#593](httprb/http#593)
  [#592](httprb/http#592)
  Support informational (1XX) responses.
  ([@ixti])

* [#590](httprb/http#590)
  [#589](httprb/http#589)
  Fix response headers paring.
  ([@Bonias])

* [#587](httprb/http#587)
  [#585](httprb/http#585)
  Fix redirections when server responds with multiple Location headers.
  ([@ixti])

* [#581](httprb/http#581)
  [#582](httprb/http#582)
  Add Ruby 2.7.x support.
  ([@janko])

* [#577](httprb/http#577)
  Fix `Chainable#timeout` with frozen Hash.
  ([@antonvolkoff])

* [#576](httprb/http#576)
  [#524](httprb/http#524)
  **BREAKING CHANGE**
  Preserve header names casing.
  ([@joshuaflanagan])

* [#540](httprb/http#540)
  [#538](httprb/http#538)
  **BREAKING CHANGE**
  Require explicit MIME type for Response#parse
  ([@ixti])

* [#532](httprb/http#532)
  Fix pipes support in request bodies.
  ([@ixti])

* [#530](httprb/http#530)
  Improve header fields name/value validation.
  ([@Bonias])

* [#506](httprb/http#506)
  [#521](httprb/http#521)
  Skip auto-deflate when there is no body.
  ([@Bonias])

* [#489](httprb/http#489)
  Fix HTTP parser.
  ([@ixti], [@fxposter])

* [#546](httprb/http#546)
  **BREAKING CHANGE**
  Provide initiating `HTTP::Request` object on `HTTP::Response`.
  ([@joshuaflanagan])

* [#571](httprb/http#571)
  Drop Ruby 2.3.x support.
  ([@ixti])

* [3ed0c31](httprb/http@3ed0c31)
  Drop Ruby 2.4.x support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get the request object from the response?
4 participants