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

em-http-request middlewares get double-called #899

Closed
ajvondrak opened this issue Aug 23, 2020 · 1 comment · Fixed by #936
Closed

em-http-request middlewares get double-called #899

ajvondrak opened this issue Aug 23, 2020 · 1 comment · Fixed by #936

Comments

@ajvondrak
Copy link
Contributor

Specifically, the #request methods are invoked in two places.

  1. em-http-request calls them from EventMachine::HttpClient#connection_completed: https://github.com/igrigorik/em-http-request/blob/157d5ff281c503656192825c388b28e7f35e04ce/lib/em-http/client.rb#L57-L59
  2. webmock calls them from EventMachine::WebMockHttpClient#build_request_signature:
    @conn.middleware.select {|m| m.respond_to?(:request) }.each do |m|
    headers, body = m.request(self, headers, body)
    end

The request signature is built at the start of EventMachine::WebMockHttpConnection#activate_connection here:

request_signature = client.request_signature

This happens prior to duplicating the logic of EventMachine::HttpConnection#activate_connection (ref) to call #finalize_request here:

post_init
@deferred = false
@conn = conn
conn.parent = self
conn.pending_connect_timeout = @connopts.connect_timeout
conn.comm_inactivity_timeout = @connopts.inactivity_timeout
finalize_request(client)

Then the #finalize_request call hits the superclass EventMachine::HttpConnection#finalize_request, which adds a callback to EventMachine::HttpClient#connection_completed: https://github.com/igrigorik/em-http-request/blob/157d5ff281c503656192825c388b28e7f35e04ce/lib/em-http/http_connection.rb#L159

Which, as noted at the start, runs the #request middleware again.

The middleware handling was added waaay back in #111. Since the #request hook can amend the request headers & body, it makes sense that you'd want it to run before building & checking the webmock signature. But the circuitous control flow means that if the middleware has a side-effect (e.g., adding another callback), those side-effects will get triggered twice, which can lead to some really bizarre (and incorrect) behavior in the test suite.

@bblimke
Copy link
Owner

bblimke commented Sep 11, 2020

@ajvondrak thank you for investigating this and presenting the detailed explanation of the problem. Since you know the problem, do you perhaps also know how this can be fixed? If yes, then would you fancy submitting a PR with the fix?

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 a pull request may close this issue.

2 participants