test: prevent handling in-flight requests after worker was stopped

[diego-aquino]

I was investigating #2597 and comparing the reproduction repository with MSW's test suite. The reproduction tests frequently fail due to requests still being handled after the worker was stopped.

However, the existing MSW test checking this behavior does not appear to be flaky:

msw/test/browser/msw-api/setup-worker/stop/in-flight-request.test.ts

Line 31 in 5ab5857

test('bypasses requests made after the worker was stopped', async ({

I've created two new test cases to better understand the problem:

1. bypasses requests not immediately made after the worker was stopped, considering no default response

   This test stops the worker and makes a request in different page.evaluate calls. It checks if the request was bypassed by expecting a Cannot GET response text. From my local executions, it always passes.

2. bypasses requests immediately made after the worker was stopped, considering no default response

   This test stops the worker and makes a request in the same page.evaluate call. Similarly to test 1, it expects a Cannot GET response text, but it shows the same flaky behavior as the reproduction tests, although less frequently.

Here's an example of a failure:

1) [chromium] › test/browser/msw-api/setup-worker/stop/in-flight-request.test.ts:67:3 ›
   bypasses requests immediately made after the worker was stopped, considering no default response (40) 

  Error: expect(received).toContain(expected) // indexOf

  Expected substring: "Cannot GET /resource"
  Received string:    "hello world"

     98 |     })
     99 |
  > 100 |     expect(data).toContain('Cannot GET /resource')
        |                  ^
    101 |   })
    102 | }
    103 |

Note

I've added a for loop to repeat the new tests and make them more likely to fail. Still, not all test executions cause failures. Run them a couple of times if they continue to pass.

pnpm test:browser in-flight-request.test.ts

Maybe the overhead of page.evaluate is masking the issue in the existing test?

---

This pull request does not yet contain any changes to the source code. I've only added tests trying to simulate the issue.

@kettanaito, as I'm new to the codebase, I'm not yet sure about how to fix this. I'm willing to contribute after hearing your feedback and possible courses of action. Thanks!

Closes #2597.

[kettanaito]

Hey, @diego-aquino. Thanks for adding these tests!

Yes, page.evaluate definitely adds to the behaviors here. As any async logic, it makes things more/less reproducible depending on how you look. worker.stop() is synchronous, but it routes through the worker, which makes its effect asynchronous. I suppose that is the problematic window during which in-flight requests might still get handled.

I highly recommend put this on hold until #2650 is merged. It contains fundamental changes to the internal architecture and how the worker is handled, in particular. While it won't fix this issue, it would be easier to branch the fix from there.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5a8c3bc5-8d77-43e4-9a13-d0e2c0bcb14c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}