multiple interceptors causing restore problem

[marcosvega91]

We know that there is a problem when using multiple interceptors. restore function will remove monkey patches even if there are other interceptors. The last one is mswjs/msw#637

The solution of this problem is not really easy but what you think if we add a counter on the interceptor? I know that is not a complete solution but could be a first start

--- a/src/createInterceptor.ts
+++ b/src/createInterceptor.ts
@@ -63,12 +63,15 @@ export interface InterceptorApi {
   restore(): void
 }
 
+let interceptosCounter = 0
+
 export function createInterceptor(options: InterceptorOptions): InterceptorApi {
   const observer = new StrictEventEmitter<InterceptorEventsMap>()
   let cleanupFns: InterceptorCleanupFn[] = []
 
   return {
     apply() {
+      interceptosCounter++
       cleanupFns = options.modules.map((interceptor) => {
         return interceptor(observer, options.resolver)
       })
@@ -78,14 +81,16 @@ export function createInterceptor(options: InterceptorOptions): InterceptorApi {
     },
     restore() {
       observer.removeAllListeners()
-
+      interceptosCounter--
       if (cleanupFns.length === 0) {
         throw new Error(
           `Failed to restore patched modules: no patches found. Did you forget to run ".apply()"?`
         )
       }
 
-      cleanupFns.forEach((restore) => restore())
+      if (interceptosCounter === 0) {
+        cleanupFns.forEach((restore) => restore())
+      }
     },
   }
 }

[kettanaito]

Hey. Thanks for opening this issue.

Does this also cause mswjs/msw#474?

Keeping a counter doesn't guarantee the proper cleanup phase, as the patches are applied per process. The code above does nothing if interceptorsCounter is > 0, so calling .restore() doesn't restore any patches. That doesn't seem right.

If it's a question of identifying which exact patches to restore, each call to apply() can generate a UUID and each call to .restore() could call the cleanup of the interceptor associated with that particular UUID. That still doesn't solve the issue that modules are patched per process, so when you restore http for one UUID that restores it for all interceptors.

[marcosvega91]

Yes but the main idea is why we should restore something if it's still used by other ?

[kettanaito]

It's deterministic to restore patches when you call .restore(). It may be confusing for the users to experience that calling .restore() doesn't actually revert the patches, but awaits until there was a sufficient amount of calls to do so.

We should follow a deterministic pattern: 1 .apply() corresponds to 1 .restore(). Both methods should do something.

I'm reluctant when it comes to building a custom logic around that, because it doesn't solve the actual problem: context leak in concurrent mode. We may implement a guard to ensure .apply() isn't called multiple times while it hasn't been restored. Overall, the latest API forbids multiple request handlers, so it may be a partial fix for this.

-interceptor.use(handlerA)
-interceptor.use(handlerB)
+const interceptor = createInterceptor({
+  resolver: singleHandler
+})
+interceptor.apply()

If we could scope the calls to .apply() by some criteria (i.e. process ID), then we could keep a map of the patches applied per process, restoring only the patches in that particular process.

[kettanaito]

I've redesigned interceptors in #232 and now any interceptor can be instantiated multiple times. They are also disposed of correctly. Once the feat/events is merged, the new architecture will be released.

[kettanaito]

Released: v0.15.2 🎉

This has been released in v0.15.2!

• 📄 Release notes
• 📦 npm package

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.

---

Predictable release automation by @ossjs/release.