fix(common): canceled JSONP requests won't throw console error with missing callback function (#36807)

martinsik · thePunderWoman · commit 64da1daa7827 · 2022-02-24T17:24:34.000Z
This commit fixes a use-case where unsubscribing from a JSONP request will result in "Uncaught ReferenceError: ng_jsonp_callback_xy is not defined" thrown into console. Unsubscribing won't remove its associated callback function because the requested script will finish loading anyway and will try to call the handler. PR Close #34818 PR Close #36807
diff --git a/packages/common/http/src/jsonp.ts b/packages/common/http/src/jsonp.ts
@@ -125,15 +125,17 @@ export class JsonpClientBackend implements HttpBackend {
       // cleanup() is a utility closure that removes the <script> from the page and
       // the response callback from the window. This logic is used in both the
       // success, error, and cancellation paths, so it's extracted out for convenience.
-      const cleanup = () => {
+      const cleanup = (removeCallback = true) => {
         // Remove the <script> tag if it's still on the page.
         if (node.parentNode) {
           node.parentNode.removeChild(node);
         }
 
-        // Remove the response callback from the callbackMap (window object in the
-        // browser).
-        delete this.callbackMap[callback];
+        if (removeCallback) {
+          // Remove the response callback from the callbackMap (window object in the
+          // browser).
+          delete this.callbackMap[callback];
+        }
       };
 
       // onLoad() is the success callback which runs after the response callback
@@ -218,7 +220,9 @@ export class JsonpClientBackend implements HttpBackend {
         node.removeEventListener('error', onError);
 
         // And finally, clean up the page.
-        cleanup();
+        // Unsubscription won't remove the callback handler because there's no way to stop
+        // loading <script> once it has been added to DOM.
+        cleanup(false);
       };
     });
   }
diff --git a/packages/common/http/test/jsonp_spec.ts b/packages/common/http/test/jsonp_spec.ts
@@ -17,7 +17,6 @@ function runOnlyCallback(home: any, data: Object) {
   const keys = Object.keys(home);
   expect(keys.length).toBe(1);
   const callback = home[keys[0]];
-  delete home[keys[0]];
   callback(data);
 }
 
@@ -63,6 +62,11 @@ const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
       });
       document.mockError(error);
     });
+    it('allows the callback to be invoked when the request is cancelled', () => {
+      backend.handle(SAMPLE_REQ).subscribe().unsubscribe();
+      runOnlyCallback(home, {data: 'This is a test'});
+      expect(Object.keys(home).length).toBe(0);
+    });
     describe('throws an error', () => {
       it('when request method is not JSONP',
          () => expect(() => backend.handle(SAMPLE_REQ.clone<never>({method: 'GET'})))
