oesmith / puffing-billy

A rewriting web proxy for testing interactions between your browser and external sites. Works with ruby + rspec.
MIT License
654 stars 170 forks source link

Clear up pending work (please advise) #346

Closed pawlik closed 1 month ago

pawlik commented 2 months ago

TL;DR; Is there a way to cancel all "pending" requests waiting to be completed by the event machine's loop?

Context: we have non_whitelisted_requests_disabled = true for complete test isolation.

We've noticed that under some conditions (e.g., when the spec makes the browser send many requests) - puffing billing is still busy fulfilling those requests, even though the spec that triggered them has already been concluded.

This leads to sometimes confusing errors, showing up "near" later specs (and occasionally flaky, but forgive me if I filled in the details later).

The way we deal with that today is by stopping and starting Billy's proxy:

RSpec.configure do |config|
    proxy.stop
end

config.append_after(:each, type: :system) do
    proxy.start
end

Billy.configure do |c|
  # We also need to set a port once for the whole suite
  server = TCPServer.new("127.0.0.1", 0)
  port = server.addr[1]
  server.close
  c.proxy_port = port
end

But this seems off and obviously introduces the extra overhead of stopping and starting the server between each spec. So I came here for a piece of advice (admittedly the proper understanding of how Billy and EM work escapes me):

What would be a proper/recommended way to "tell" EM not to bother completing whatever requests there are without restarting the Billy's proxy server?

ronwsmith commented 2 months ago

What requests are hanging? If it's pixels taking longer to load, best practice is to disable those in test envs.

khamusa commented 2 months ago

@ronwsmith we use a react library in our FE application that fetches all country flags as .svg files from cloudflare CDN, and those are usually the ones leaking to the other tests - we're talking 200+ svg requests (we're looking into reducing this for the sake of stabilizing our test suite, but we're also interested in a more future proof solution, which is why we're trying to clear the "queue" as @pawlik suggested).

We've mocked these requests in our tests since we don't want to reach the CDN, but since puffing billy clears the mocks between two tests, if we get to this point and there are still flag requests queued to be processed, they end up failing because of the non_whitelisted_requests_disabled = true, causing a bunch of browser console errors to fail the other - unrelated - test.

ronwsmith commented 2 months ago

Hmm, thanks for the additional detail. I don't have any immediate ideas to "clear the queue". I would personally add some additional waiting until everything was loaded before the spec finishes, and then work to optimize the full page rendering (perhaps lazy loading the svgs in your case)

pawlik commented 1 month ago

Thanks. It feels like unfortunately puffing billy has some limitation for our current architecture (rails backend serving GQL API):

The front end first loads, and then all requests to GQL and puffing billy are handled within single process, which seems to make it quite slow to respond. At least that's my current hypothesis why we struggle.

~Puffing billy on it's own can serve a ton of requests, I checked. But it seems to be better tailored to more traditional rails apps (just living it more as an FYI for others)~ I was completely off with this guess, the reason seems to be differetn. More details here: https://github.com/oesmith/puffing-billy/issues/347