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

How to deal with requests made before msw has been activated? #73

Closed
kentcdodds opened this issue Mar 20, 2020 · 25 comments · Fixed by #90
Closed

How to deal with requests made before msw has been activated? #73

kentcdodds opened this issue Mar 20, 2020 · 25 comments · Fixed by #90

Comments

@kentcdodds
Copy link
Contributor

If I have a situation like this:

start('/msw.js')

fetch('/something-that-should-be-mocked')

MSW will not be registered as ready until after the request has already been forwarded along. What do you recommend for situations like this where the app is trying to make requests immediately?

@kentcdodds
Copy link
Contributor Author

For anyone coming along, here's the basic gist of my current workaround:

window.serverReady = start('/msw.js')

async function bootstrapAppData() {
  await window.serverReady
  await fetch('/something-that-should-be-mocked')
  // etc...
}

In production I don't think this should be a problem because you'd basically just await undefined which is nbd.

@kettanaito
Copy link
Member

I'd recommend to start the Service Worker as early as possible in your app. I usually put my mocks.js in the root of my app:

import React from 'react'
import ReactDOM from 'react-dom'
import App from './App'
import './mocks'

ReactDOM.render(<App />, rootElement)

This implies I use composeMocks() and invoke the start() function within mocks.js.

However, since Service Worker activation is an async operation there is still a race condition in place of what happens first: your app's mount and potential data fetching, or MSW activation. To eliminate this race condition you can listen to the Promise returned from calling the start() function:

// mocks.js
import { composeMocks } from 'msw'

return composeMocks(/* mock definition */)
// index.js
import { start } from './mocks'

async function bootstrapApp() {
  await start()
  ReactDOM.render(<App />, rootElement)
}
bootstrapApp()

It is up to the user to define at which level awaiting the start() Promise is applicable, depending on their situation. One of the core principles of MSW is the absence of changes to the existing code, that's why I generally don't like the idea of making your app's logic dependent on the MSW activation status.

@kentcdodds
Copy link
Contributor Author

Yep, I think that's the best that can be done here. Thanks.

@kentcdodds
Copy link
Contributor Author

kentcdodds commented Mar 27, 2020

This should be re-opened. The promise returned by start can resolve before the service worker is actually ready.

You can observe this in my app which has MSW deployed:

  1. https://5e7d73d0938fb965a65bbdf3--the-react-bookshelf.netlify.app
  2. Register with username abc and password 123
  3. Do a "hard refresh" via + + r
  4. Notice error in the app and console: api-client.js:20 GET https://bookshelf.jk/api/me net::ERR_NAME_NOT_RESOLVED

I've been able to workaround this by adding a timeout of 100ms before making any requests. I need some way to wait until the service worker has not only installed but is also ready to start proxying requests.

@kentcdodds kentcdodds reopened this Mar 27, 2020
@kettanaito
Copy link
Member

kettanaito commented Mar 27, 2020

@kentcdodds I think this has to do with the fact that Service Worker activation and activation of mocking are two separate subsequent events. The promise returned from start() is, as you've mentioned, resolves the Service Worker activation, but the actual signal that client-side library sends to the worker to enable the mocking follows the activation.

This is doable, we just need to decide a proper way for that mocking activation to propagate to the client-side. Thanks for bumping this.

@kentcdodds
Copy link
Contributor Author

Awesome! Can't wait to try this 👏 thanks!

@kettanaito
Copy link
Member

@kentcdodds, I plan to release it in 0.11.5 today. This will bump the Service Worker's integrity (changes done to mockServiceWorker.js file on the library's side), so please update the Service Worker according to the instructions and it should be fine.

Let me know if this changes resolves the activation Promise in time for you.

@kentcdodds
Copy link
Contributor Author

Oh! I thought this was already released 😂 I already updated and removed my workaround. I guess I'll add it back for now 😅

@kentcdodds
Copy link
Contributor Author

Actually, from the look of my node modules I think this was already released 🤔

@kettanaito
Copy link
Member

@kentcdodds should be present in 0.11.5 only (see 6ffeb7e). Previously the start() function also returned a Promise, but it resolved when the Service Worker was registered, and not necessarily ready to mock requests. Now it should resolve only when it signaled back to the client-side library that the mocking has been enabled.

@kentcdodds
Copy link
Contributor Author

Awesome 👍 I have 0.11.5

Thank you a lot! I'm giving a workshop right now using this. We haven't gotten to the data part yet, but I'm excited about it. Thank you!

@kettanaito
Copy link
Member

I'm glad to indirectly contribute to your workshop through MSW! I hope the workshop goes the best way. I've also noticed you've used some parts of the library in fetch-mock.js, which made me thinking whether MSW should expose some internals in a consumable way. If you have any thoughts on this, feel free to share.

Thanks for using MSW. Means a lot.

@kentcdodds
Copy link
Contributor Author

kentcdodds commented Mar 31, 2020 via email

@mitchconquer
Copy link

mitchconquer commented May 6, 2020

Hi! I am following Kent Dodds' bookshelf app app and having troubles when the app tries to "bootstrap" a signed in user if they refresh the page. (It sends a request out to get the user's profile as soon as the app is mounted, like in Kent's bootstrap example.)

I am firing up MSW in the app's index.js file

  1. I'm starting the mock server
  2. Assigning the return value of start() to a property on the window object (window.__mockServerReady = start();)

Then, I have an apiClient.js that handles all fetch calls. Before it actually fetches, it awaits the resolution of the promise on the window but it seems MSW does not instantiating in time.

Here's what I have before any fetch requests are made:

async function apiClient(endpoint, config) {

 if (process.env.NODE_ENV === 'development') {
    // If local development, wait for the MSW mock API to fire up
    const registration = await window.__mockServerReady; // 👈wait for MSW to fire up

    console.log(
      'registration instanceof ServiceWorkerRegistration',
      registration instanceof ServiceWorkerRegistration 
    );

    console.log(JSON.stringify(window.__merchantMockServerReady));
  }
  
  // ... setup fetch request

  console.log('prefetch', endpoint);

 const res = await window.fetch(
    `${API_URL_BASE}/${API_VERSION}/${endpoint}`,
    config
  );

  console.log('postfetch', endpoint);
}

And this is what I see in the console

image

If I add a 500ms delay in the apiClient before the fetch, it works.

async function apiClient(endpoint, config) {

 if (process.env.NODE_ENV === 'development') {
    // If local development, wait for the MSW mock API to fire up
    const registration = await window.__mockServerReady;

    console.log(
      'registration instanceof ServiceWorkerRegistration',
      registration instanceof ServiceWorkerRegistration 
    );

    // 👇 Add this 500ms timeout and the initial "user session bootstrap" call works
    if (!(registration instanceof ServiceWorkerRegistration)) {
      await new Promise((r) => setTimeout(r, 500));
    }

    console.log(JSON.stringify(window.__merchantMockServerReady));
  }
  
  // ... setup fetch request

  console.log('prefetch', endpoint);

 const res = await window.fetch(
    `${API_URL_BASE}/${API_VERSION}/${endpoint}`,
    config
  );

  console.log('postfetch', endpoint);
}

If I load the page so that it does not have the initial user session bootstrap call, it seems to load well

image

Update

I noticed that if I add an additional check to my bootstrap file before I make the call out to my apiClient it also works without the 500ms delay.

    if (process.env.NODE_ENV === 'development') {
      // If local development, wait for the MSW mock API to fire up
      await window.__mockServerReady;
    }

It made me realize that there is timing issue. I then moved the call to start bootstrapping inside my React.useLayoutEffect function so that it only calls once that component is mounted. That also works without the additional timeout or the second await of window.__mockServerReady.

Kent has his bookshelf app work the first way, with the call outside the React component, but there is probably some subtle wizardry I'm missing to allow for that 😅

So, I found a workaround but not sure if my initial point of the await window.__mockServerReady not working still stands.

/Update

My apologies that this format for reporting the error isn't ideal but I haven't figured out how to create a codesandbox.com example with this lib yet. Please let me know if there's a more helpful format to report errors.

Thanks again for your work on this library! I think it's really awesome!

@kettanaito
Copy link
Member

kettanaito commented May 6, 2020

Hey, @mitchconquer. Please, don't worry, this format is perfectly fine. It's my bad, I haven't yet established a way to bootstrap scenarios with MSW in a sandbox form.

Regarding the issue, I can recommend to store the start function itself, rather than its return statement. In case there's some dark magic in place, it'll help us ensure that we call the start() at the expected time.

// mocks.js
const worker = setupWorker(...)
window.__mswStart = worker.start // notice the reference
// apiClient.js
async function apiClient() {
  if (__DEV__) {
    await window.__mswStart()
  }
}

I'd be surprised if this didn't work. I have a few integration tests that rely on awaiting the start() Promise, so such setup should be a valid one.

Let me know if this suggestion is of any help. I'd prefer if you don't have to introduce timeouts to await the registration.

Going back to sandboxing examples, I've tried to establish a template in Codesandbox, yet for some reason it fails to register the worker. To be more precise, it ignores the registration attempt, when I reference the service worker file relatively. If I give it an absolute URL (that includes the sandbox URL), it registers just fine. Haven't figured a reliable sandbox template just yet.

@mitchconquer
Copy link

Hi! Thank you for the suggestion!

I implemented it and see that the function was not set to the variable on window so I think my load order was the problem. Even though the MSW file was required before the rest of the app, it was not running until after the bootstrap code. I'm not sure why I didn't think of that 😅

I'm trying to find resources to explain why but in the meantime, requiring the MSW code at the top of my bootstrap file did the trick.

Thanks again! 😸

@mitchconquer
Copy link

To give an update: it does still fail if I hard refresh/reload but it works great otherwise.

@kettanaito
Copy link
Member

I'm sorry to hear that. Could you please describe in more details what happens during the hard reload? Also, what is the browser you are using?

I've tried to tackle hard reload previously (#98, #122), in case it's not fixed yet I'd love to find out more details (sounds like an interesting case).

@kettanaito
Copy link
Member

Anybody reading this now: MSW hijacks all requests that happen between calling setupWorker().start() and the actual activation of the Service Worker. You don't have to treat those request in any way, it's an automatic behavior to capture and defer those requests until the mocking is ready.

@msutkowski
Copy link
Member

@kettanaito Thanks for this awesome lib! I've been looking everywhere for how to get this running in a codesandbox. I've come up with this, but I feel like it's probably very brittle. Any guidance on the best implementation for this would be greatly appreciated. Also, please let me know if this isn't the right issue to comment on 😅

worker.start();
navigator.serviceWorker.register(
  `${window.location.origin}/mockServiceWorker.js`
);

Source sandbox: https://codesandbox.io/s/rtk-crud-entities-typed-kuwlp?file=/src/index.tsx

@kettanaito
Copy link
Member

Hey, @msutkowski. Thanks for the kind words!

If you wish to change the Service Worker script URL, consider using the serviceWorker.url option of the .start() method. The .start() method does Service Worker registration and activation (mock readiness). I would discourage trying to register the mock Service Worker manually, as this is not enough to ensure it's operable.

Last time I tried to set up a Codesandbox example I had an issue that it completely ignored a relative path to the worker, even if it's a valid path. The only way I got it to work is by supplying an absolute path to serviceWorker.url option. However, even in that case it still didn't intercept requests, as far as I remember.

Please let me know if you get it to run in sandbox, we could use your experience to establish an official sandbox example. Thanks.

@sjbuysse
Copy link

sjbuysse commented Nov 9, 2021

Anybody reading this now: MSW hijacks all requests that happen between calling setupWorker().start() and the actual activation of the Service Worker. You don't have to treat those request in any way, it's an automatic behavior to capture and defer those requests until the mocking is ready.

Is this still true @kettanaito ?
I have a project with msw @ 0.35.0 , and my initial requests are going through to the backend, only the (same) requests made later are actually intercepted.

@kettanaito
Copy link
Member

kettanaito commented Nov 9, 2021

It's true but, apparently, we cannot account for all asynchronicity entirely. I recommend following the deferred mounting strategy for reliable results.

We could also use some help debugging why some requests are not captured anyway, given we're using "deferNetworkRequest" function.

@divStar
Copy link

divStar commented Nov 30, 2021

I apologize to barge in like that, but I've been encountering this problem when using MSW with Vue 3 (+TypeScript).

Also first of all I'd like to thank you @kettanaito for such an excellent and really easy-to-use non-intrusive backend-mock-library 😃. I am only using this for a home project to get to know Vue and Go (with only vague outcome set), but seeing how nice and easy, yet customizable it is, perhaps I should ask if we should give this a try at work, too, because using extensive proxy layers can be exhausting.

But I digress... I initially was not able to await worker.start(...) in main.ts, because I did not want to change the target property in tsconfig to es2017 or higher and configure webpack to support top-level await via experiments.topLevelAwait.

So I decided to do this instead:

async function prepareWorkers(): Promise<void> {
  if (process.env.NODE_ENV) {
    await repositoryWorker.start({ ... });
  }
}

prepareWorkers().then(() => {
  createApp(App).mount("#app");
});

This basically allowed me to either start workers and wait for them to be started and mocking to become available (for development mode). If the software is running in any other environment, I'd expect prepareWorkers to instantly return and proceed with the main application, though I have not tried it yet.

It might also not be a good idea to completely ignore the onrejected Promise outcome, but I did not care enough to set it atm.

Again: thank you very much! I am playing around with it.

@kettanaito
Copy link
Member

Thank you for your kind words, @divStar. Your approach makes sense, that's exactly what we suggest in the Deferred mounting recipe in our documentation.

It might also not be a good idea to completely ignore the onrejected Promise outcome, but I did not care enough to set it atm.

I wouldn't handle any registration promise rejections. That way any rejection is propagated to the upper scope as an exception, failing the build.

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.

6 participants