fix(loader): Improve loader callback handling (#58070)

This slightly refactors the loader script and clarifies function/variable naming etc.

---------

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>

src/sentry/templates/sentry/js-sdk-loader.js.tmpl

@@ -1,6 +1,5 @@
-{% load sentry_helpers %}(function sentryLoader(_window, _document, _errorEvent, _unhandledrejectionEvent, _namespace, _publicKey, _sdkBundleUrl, _config, _lazy) {
+{% load sentry_helpers %}(function sentryLoader(_window, _document, _errorEvent, _unhandledrejectionEvent, _namespace, _publicKey, _sdkBundleUrl, _loaderInitConfig, _lazy) {
     var lazy = _lazy;
-    var forceLoad = false;
     for (var i = 0; i < document.scripts.length; i++) {
         if (document.scripts[i].src.indexOf(_publicKey) > -1) {
             // If lazy was set to true above, we need to check if the user has set data-lazy="no"
@@ -11,35 +10,40 @@
             break;
         }
     }
-    var injected = false;
     var onLoadCallbacks = [];
+    function queueIsError(item) {
+        return 'e' in item;
+    }
+    function queueIsPromiseRejection(item) {
+        return 'p' in item;
+    }
+    function queueIsFunction(item) {
+        return 'f' in item;
+    }
+    var queue = [];
     // Create a namespace and attach function that will store captured exception
     // Because functions are also objects, we can attach the queue itself straight to it and save some bytes
-    var queue = function (content) {
-        // content.e = error
-        // content.p = promise rejection
-        // content.f = function call the Sentry
-        if (('e' in content ||
-            'p' in content ||
-            (content.f && content.f.indexOf('capture') > -1) ||
-            (content.f && content.f.indexOf('showReportDialog') > -1)) &&
-            lazy) {
+    function enqueue(item) {
+        if (lazy &&
+            (queueIsError(item) ||
+                queueIsPromiseRejection(item) ||
+                (queueIsFunction(item) && item.f.indexOf('capture') > -1) ||
+                (queueIsFunction(item) && item.f.indexOf('showReportDialog') > -1))) {
             // We only want to lazy inject/load the sdk bundle if
             // an error or promise rejection occured
             // OR someone called `capture...` on the SDK
-            injectSdk(onLoadCallbacks);
+            injectCDNScriptTag();
         }
-        queue.data.push(content);
-    };
-    queue.data = [];
+        queue.push(item);
+    }
     function onError() {
         // Use keys as "data type" to save some characters"
-        queue({
+        enqueue({
             e: [].slice.call(arguments),
         });
     }
     function onUnhandledRejection(e) {
-        queue({
+        enqueue({
             p: 'reason' in e
                 ? e.reason
                 : 'detail' in e && 'reason' in e.detail
@@ -47,48 +51,59 @@
                     : e,
         });
     }
-    function injectSdk(callbacks) {
-        if (injected) {
+    function onSentryCDNScriptLoaded() {
+        try {
+            // Add loader as SDK source
+            _window.SENTRY_SDK_SOURCE = 'loader';
+            var SDK_1 = _window[_namespace];
+            var cdnInit_1 = SDK_1.init;
+            // Configure it using provided DSN and config object
+            SDK_1.init = function (options) {
+                // Remove the lazy mode error event listeners that we previously registered
+                // Once we call init, we can assume that Sentry has added it's own global error listeners
+                _window.removeEventListener(_errorEvent, onError);
+                _window.removeEventListener(_unhandledrejectionEvent, onUnhandledRejection);
+                var mergedInitOptions = _loaderInitConfig;
+                for (var key in options) {
+                    if (Object.prototype.hasOwnProperty.call(options, key)) {
+                        mergedInitOptions[key] = options[key];
+                    }
+                }
+                setupDefaultIntegrations(mergedInitOptions, SDK_1);
+                cdnInit_1(mergedInitOptions);
+            };
+            // Wait a tick to ensure that all `Sentry.onLoad()` callbacks have been registered
+            setTimeout(function () { return setupSDK(SDK_1); });
+        }
+        catch (o_O) {
+            console.error(o_O);
+        }
+    }
+    var injectedCDNScriptTag = false;
+    /**
+     * Injects script tag into the page pointing to the CDN bundle.
+     */
+    function injectCDNScriptTag() {
+        if (injectedCDNScriptTag) {
             return;
         }
-        injected = true;
+        injectedCDNScriptTag = true;
         // Create a `script` tag with provided SDK `url` and attach it just before the first, already existing `script` tag
         // Scripts that are dynamically created and added to the document are async by default,
         // they don't block rendering and execute as soon as they download, meaning they could
         // come out in the wrong order. Because of that we don't need async=1 as GA does.
         // it was probably(?) a legacy behavior that they left to not modify few years old snippet
         // https://www.html5rocks.com/en/tutorials/speed/script-loading/
-        var _currentScriptTag = _document.scripts[0];
-        var _newScriptTag = _document.createElement('script');
-        _newScriptTag.src = _sdkBundleUrl;
-        _newScriptTag.crossOrigin = 'anonymous';
+        var firstScriptTagInDom = _document.scripts[0];
+        var cdnScriptTag = _document.createElement('script');
+        cdnScriptTag.src = _sdkBundleUrl;
+        cdnScriptTag.crossOrigin = 'anonymous';
         // Once our SDK is loaded
-        _newScriptTag.addEventListener('load', function () {
-            try {
-                _window.removeEventListener(_errorEvent, onError);
-                _window.removeEventListener(_unhandledrejectionEvent, onUnhandledRejection);
-                // Add loader as SDK source
-                _window.SENTRY_SDK_SOURCE = 'loader';
-                var SDK_1 = _window[_namespace];
-                var oldInit_1 = SDK_1.init;
-                // Configure it using provided DSN and config object
-                SDK_1.init = function (options) {
-                    var target = _config;
-                    for (var key in options) {
-                        if (Object.prototype.hasOwnProperty.call(options, key)) {
-                            target[key] = options[key];
-                        }
-                    }
-                    setupDefaultIntegrations(target, SDK_1);
-                    oldInit_1(target);
-                };
-                sdkLoaded(callbacks, SDK_1);
-            }
-            catch (o_O) {
-                console.error(o_O);
-            }
+        cdnScriptTag.addEventListener('load', onSentryCDNScriptLoaded, {
+            once: true,
+            passive: true,
         });
-        _currentScriptTag.parentNode.insertBefore(_newScriptTag, _currentScriptTag);
+        firstScriptTagInDom.parentNode.insertBefore(cdnScriptTag, firstScriptTagInDom);
     }
     // We want to ensure to only add default integrations if they haven't been added by the user.
     function setupDefaultIntegrations(config, SDK) {
@@ -115,48 +130,46 @@
             __sentry.hub &&
             __sentry.hub.getClient());
     }
-    function sdkLoaded(callbacks, SDK) {
+    function setupSDK(SDK) {
         try {
             // We have to make sure to call all callbacks first
-            for (var i = 0; i < callbacks.length; i++) {
-                if (typeof callbacks[i] === 'function') {
-                    callbacks[i]();
+            for (var i = 0; i < onLoadCallbacks.length; i++) {
+                if (typeof onLoadCallbacks[i] === 'function') {
+                    onLoadCallbacks[i]();
                 }
             }
-            var data = queue.data;
-            var initAlreadyCalled = sdkIsLoaded();
-            // Call init first, if provided
-            data.sort(function (a) { return (a.f === 'init' ? -1 : 0); });
-            // We want to replay all calls to Sentry and also make sure that `init` is called if it wasn't already
-            // We replay all calls to `Sentry.*` now
-            var calledSentry = false;
-            for (var i = 0; i < data.length; i++) {
-                if (data[i].f) {
-                    calledSentry = true;
-                    var call = data[i];
-                    if (initAlreadyCalled === false && call.f !== 'init') {
-                        // First call always has to be init, this is a conveniece for the user so call to init is optional
-                        SDK.init();
-                    }
-                    initAlreadyCalled = true;
-                    SDK[call.f].apply(SDK, call.a);
+            // First call all inits from the queue
+            for (var i = 0; i < queue.length; i++) {
+                var item = queue[i];
+                if (queueIsFunction(item) && item.f === 'init') {
+                    SDK.init.apply(SDK, item.a);
                 }
             }
-            if (initAlreadyCalled === false && calledSentry === false) {
-                // Sentry has never been called but we need Sentry.init() so call it
+            // If the SDK has not been called manually, either in an onLoad callback, or somewhere else,
+            // we initialize it for the user
+            if (!sdkIsLoaded()) {
                 SDK.init();
             }
-            // Because we installed the SDK, at this point we have an access to TraceKit's handler,
+            // Now, we _know_ that the SDK is initialized, and can continue with the rest of the queue
+            // Because we installed the SDK, at this point we can assume that the global handlers have been patched
             // which can take care of browser differences (eg. missing exception argument in onerror)
-            var tracekitErrorHandler = _window.onerror;
-            var tracekitUnhandledRejectionHandler = _window.onunhandledrejection;
-            // And now capture all previously caught exceptions
-            for (var i = 0; i < data.length; i++) {
-                if ('e' in data[i] && tracekitErrorHandler) {
-                    tracekitErrorHandler.apply(_window, data[i].e);
+            var sentryPatchedErrorHandler = _window.onerror;
+            var sentryPatchedUnhandledRejectionHandler = _window.onunhandledrejection;
+            for (var i = 0; i < queue.length; i++) {
+                var item = queue[i];
+                if (queueIsFunction(item)) {
+                    // We already called all init before, so just skip this
+                    if (item.f === 'init') {
+                        continue;
+                    }
+                    SDK[item.f].apply(SDK, item.a);
+                }
+                else if (queueIsError(item) && sentryPatchedErrorHandler) {
+                    sentryPatchedErrorHandler.apply(_window, item.e);
                 }
-                else if ('p' in data[i] && tracekitUnhandledRejectionHandler) {
-                    tracekitUnhandledRejectionHandler.apply(_window, [data[i].p]);
+                else if (queueIsPromiseRejection(item) &&
+                    sentryPatchedUnhandledRejectionHandler) {
+                    sentryPatchedUnhandledRejectionHandler.apply(_window, [item.p]);
                 }
             }
         }
@@ -167,19 +180,17 @@
     // We make sure we do not overwrite window.Sentry since there could be already integrations in there
     _window[_namespace] = _window[_namespace] || {};
     _window[_namespace].onLoad = function (callback) {
-        onLoadCallbacks.push(callback);
-        if (lazy && !forceLoad) {
+        // If the SDK was already loaded, call the callback immediately
+        if (sdkIsLoaded()) {
+            callback();
             return;
         }
-        injectSdk(onLoadCallbacks);
+        onLoadCallbacks.push(callback);
     };
     _window[_namespace].forceLoad = function () {
-        forceLoad = true;
-        if (lazy) {
-            setTimeout(function () {
-                injectSdk(onLoadCallbacks);
-            });
-        }
+        setTimeout(function () {
+            injectCDNScriptTag();
+        });
     };
     [
         'init',
@@ -192,14 +203,14 @@
         'showReportDialog',
     ].forEach(function (f) {
         _window[_namespace][f] = function () {
-            queue({ f: f, a: arguments });
+            enqueue({ f: f, a: arguments });
         };
     });
     _window.addEventListener(_errorEvent, onError);
     _window.addEventListener(_unhandledrejectionEvent, onUnhandledRejection);
     if (!lazy) {
         setTimeout(function () {
-            injectSdk(onLoadCallbacks);
+            injectCDNScriptTag();
         });
     }
 })(window, document, 'error', 'unhandledrejection', 'Sentry', '{{ publicKey|safe }}', '{{ jsSdkUrl|safe }}', {{ config|to_json|safe }}, {{ isLazy|safe|lower }});

src/sentry/templates/sentry/js-sdk-loader.ts

@@ -11,11 +11,10 @@ declare const __LOADER__IS_LAZY__: any;
   _namespace,
   _publicKey,
   _sdkBundleUrl,
-  _config,
+  _loaderInitConfig,
   _lazy
 ) {
   let lazy = _lazy;
-  let forceLoad = false;
 
   for (let i = 0; i < document.scripts.length; i++) {
     if (document.scripts[i].src.indexOf(_publicKey) > -1) {
@@ -28,40 +27,57 @@ declare const __LOADER__IS_LAZY__: any;
     }
   }
 
-  let injected = false;
   const onLoadCallbacks: (() => void)[] = [];
 
+  // A captured error
+  type ErrorQueueItem = {e: any};
+  // A captured promise rejection
+  type PromiseRejectionQueueItem = {p: any};
+  // A captured function call to Sentry
+  type FunctionQueueItem = {a: IArguments; f: string};
+  type QueueItem = ErrorQueueItem | PromiseRejectionQueueItem | FunctionQueueItem;
+
+  function queueIsError(item: QueueItem): item is ErrorQueueItem {
+    return 'e' in item;
+  }
+
+  function queueIsPromiseRejection(item: QueueItem): item is PromiseRejectionQueueItem {
+    return 'p' in item;
+  }
+
+  function queueIsFunction(item: QueueItem): item is FunctionQueueItem {
+    return 'f' in item;
+  }
+
+  const queue: QueueItem[] = [];
+
   // Create a namespace and attach function that will store captured exception
   // Because functions are also objects, we can attach the queue itself straight to it and save some bytes
-  const queue = function (content) {
-    // content.e = error
-    // content.p = promise rejection
-    // content.f = function call the Sentry
+  function enqueue(item: QueueItem) {
     if (
-      ('e' in content ||
-        'p' in content ||
-        (content.f && content.f.indexOf('capture') > -1) ||
-        (content.f && content.f.indexOf('showReportDialog') > -1)) &&
-      lazy
+      lazy &&
+      (queueIsError(item) ||
+        queueIsPromiseRejection(item) ||
+        (queueIsFunction(item) && item.f.indexOf('capture') > -1) ||
+        (queueIsFunction(item) && item.f.indexOf('showReportDialog') > -1))
     ) {
       // We only want to lazy inject/load the sdk bundle if
       // an error or promise rejection occured
       // OR someone called `capture...` on the SDK
-      injectSdk(onLoadCallbacks);
+      injectCDNScriptTag();
     }
-    queue.data.push(content);
-  };
-  queue.data = [];
+    queue.push(item);
+  }
 
   function onError() {
     // Use keys as "data type" to save some characters"
-    queue({
+    enqueue({
       e: [].slice.call(arguments),
     });
   }
 
   function onUnhandledRejection(e) {
-    queue({
+    enqueue({
       p:
         'reason' in e
           ? e.reason
@@ -71,11 +87,50 @@ declare const __LOADER__IS_LAZY__: any;
     });
   }
 
-  function injectSdk(callbacks) {
-    if (injected) {
+  function onSentryCDNScriptLoaded() {
+    try {
+      // Add loader as SDK source
+      _window.SENTRY_SDK_SOURCE = 'loader';
+
+      const SDK = _window[_namespace];
+
+      const cdnInit = SDK.init;
+
+      // Configure it using provided DSN and config object
+      SDK.init = function (options) {
+        // Remove the lazy mode error event listeners that we previously registered
+        // Once we call init, we can assume that Sentry has added it's own global error listeners
+        _window.removeEventListener(_errorEvent, onError);
+        _window.removeEventListener(_unhandledrejectionEvent, onUnhandledRejection);
+
+        const mergedInitOptions = _loaderInitConfig;
+        for (const key in options) {
+          if (Object.prototype.hasOwnProperty.call(options, key)) {
+            mergedInitOptions[key] = options[key];
+          }
+        }
+
+        setupDefaultIntegrations(mergedInitOptions, SDK);
+        cdnInit(mergedInitOptions);
+      };
+
+      // Wait a tick to ensure that all `Sentry.onLoad()` callbacks have been registered
+      setTimeout(() => setupSDK(SDK));
+    } catch (o_O) {
+      console.error(o_O);
+    }
+  }
+
+  let injectedCDNScriptTag = false;
+
+  /**
+   * Injects script tag into the page pointing to the CDN bundle.
+   */
+  function injectCDNScriptTag() {
+    if (injectedCDNScriptTag) {
       return;
     }
-    injected = true;
+    injectedCDNScriptTag = true;
 
     // Create a `script` tag with provided SDK `url` and attach it just before the first, already existing `script` tag
     // Scripts that are dynamically created and added to the document are async by default,
@@ -83,44 +138,17 @@ declare const __LOADER__IS_LAZY__: any;
     // come out in the wrong order. Because of that we don't need async=1 as GA does.
     // it was probably(?) a legacy behavior that they left to not modify few years old snippet
     // https://www.html5rocks.com/en/tutorials/speed/script-loading/
-    const _currentScriptTag = _document.scripts[0];
-    const _newScriptTag = _document.createElement('script') as HTMLScriptElement;
-    _newScriptTag.src = _sdkBundleUrl;
-    _newScriptTag.crossOrigin = 'anonymous';
+    const firstScriptTagInDom = _document.scripts[0];
+    const cdnScriptTag = _document.createElement('script') as HTMLScriptElement;
+    cdnScriptTag.src = _sdkBundleUrl;
+    cdnScriptTag.crossOrigin = 'anonymous';
 
     // Once our SDK is loaded
-    _newScriptTag.addEventListener('load', function () {
-      try {
-        _window.removeEventListener(_errorEvent, onError);
-        _window.removeEventListener(_unhandledrejectionEvent, onUnhandledRejection);
-
-        // Add loader as SDK source
-        _window.SENTRY_SDK_SOURCE = 'loader';
-
-        const SDK = _window[_namespace];
-
-        const oldInit = SDK.init;
-
-        // Configure it using provided DSN and config object
-        SDK.init = function (options) {
-          const target = _config;
-          for (const key in options) {
-            if (Object.prototype.hasOwnProperty.call(options, key)) {
-              target[key] = options[key];
-            }
-          }
-
-          setupDefaultIntegrations(target, SDK);
-          oldInit(target);
-        };
-
-        sdkLoaded(callbacks, SDK);
-      } catch (o_O) {
-        console.error(o_O);
-      }
+    cdnScriptTag.addEventListener('load', onSentryCDNScriptLoaded, {
+      once: true,
+      passive: true,
     });
-
-    _currentScriptTag.parentNode!.insertBefore(_newScriptTag, _currentScriptTag);
+    firstScriptTagInDom.parentNode!.insertBefore(cdnScriptTag, firstScriptTagInDom);
   }
 
   // We want to ensure to only add default integrations if they haven't been added by the user.
@@ -159,53 +187,53 @@ declare const __LOADER__IS_LAZY__: any;
     );
   }
 
-  function sdkLoaded(callbacks, SDK) {
+  function setupSDK(SDK) {
     try {
       // We have to make sure to call all callbacks first
-      for (let i = 0; i < callbacks.length; i++) {
-        if (typeof callbacks[i] === 'function') {
-          callbacks[i]();
+      for (let i = 0; i < onLoadCallbacks.length; i++) {
+        if (typeof onLoadCallbacks[i] === 'function') {
+          onLoadCallbacks[i]();
         }
       }
 
-      const data = queue.data;
-
-      let initAlreadyCalled = sdkIsLoaded();
-
-      // Call init first, if provided
-      data.sort(a => (a.f === 'init' ? -1 : 0));
-
-      // We want to replay all calls to Sentry and also make sure that `init` is called if it wasn't already
-      // We replay all calls to `Sentry.*` now
-      let calledSentry = false;
-      for (let i = 0; i < data.length; i++) {
-        if (data[i].f) {
-          calledSentry = true;
-          const call = data[i];
-          if (initAlreadyCalled === false && call.f !== 'init') {
-            // First call always has to be init, this is a conveniece for the user so call to init is optional
-            SDK.init();
-          }
-          initAlreadyCalled = true;
-          SDK[call.f].apply(SDK, call.a);
+      // First call all inits from the queue
+      for (let i = 0; i < queue.length; i++) {
+        const item = queue[i];
+        if (queueIsFunction(item) && item.f === 'init') {
+          SDK.init.apply(SDK, item.a);
         }
       }
-      if (initAlreadyCalled === false && calledSentry === false) {
-        // Sentry has never been called but we need Sentry.init() so call it
+
+      // If the SDK has not been called manually, either in an onLoad callback, or somewhere else,
+      // we initialize it for the user
+      if (!sdkIsLoaded()) {
         SDK.init();
       }
 
-      // Because we installed the SDK, at this point we have an access to TraceKit's handler,
+      // Now, we _know_ that the SDK is initialized, and can continue with the rest of the queue
+
+      // Because we installed the SDK, at this point we can assume that the global handlers have been patched
       // which can take care of browser differences (eg. missing exception argument in onerror)
-      const tracekitErrorHandler = _window.onerror;
-      const tracekitUnhandledRejectionHandler = _window.onunhandledrejection;
-
-      // And now capture all previously caught exceptions
-      for (let i = 0; i < data.length; i++) {
-        if ('e' in data[i] && tracekitErrorHandler) {
-          tracekitErrorHandler.apply(_window, data[i].e);
-        } else if ('p' in data[i] && tracekitUnhandledRejectionHandler) {
-          tracekitUnhandledRejectionHandler.apply(_window, [data[i].p]);
+      const sentryPatchedErrorHandler = _window.onerror;
+      const sentryPatchedUnhandledRejectionHandler = _window.onunhandledrejection;
+
+      for (let i = 0; i < queue.length; i++) {
+        const item = queue[i];
+
+        if (queueIsFunction(item)) {
+          // We already called all init before, so just skip this
+          if (item.f === 'init') {
+            continue;
+          }
+
+          SDK[item.f].apply(SDK, item.a);
+        } else if (queueIsError(item) && sentryPatchedErrorHandler) {
+          sentryPatchedErrorHandler.apply(_window, item.e);
+        } else if (
+          queueIsPromiseRejection(item) &&
+          sentryPatchedUnhandledRejectionHandler
+        ) {
+          sentryPatchedUnhandledRejectionHandler.apply(_window, [item.p]);
         }
       }
     } catch (o_O) {
@@ -217,20 +245,18 @@ declare const __LOADER__IS_LAZY__: any;
   _window[_namespace] = _window[_namespace] || {};
 
   _window[_namespace].onLoad = function (callback) {
-    onLoadCallbacks.push(callback);
-    if (lazy && !forceLoad) {
+    // If the SDK was already loaded, call the callback immediately
+    if (sdkIsLoaded()) {
+      callback();
       return;
     }
-    injectSdk(onLoadCallbacks);
+    onLoadCallbacks.push(callback);
   };
 
   _window[_namespace].forceLoad = function () {
-    forceLoad = true;
-    if (lazy) {
-      setTimeout(function () {
-        injectSdk(onLoadCallbacks);
-      });
-    }
+    setTimeout(function () {
+      injectCDNScriptTag();
+    });
   };
 
   [
@@ -244,7 +270,7 @@ declare const __LOADER__IS_LAZY__: any;
     'showReportDialog',
   ].forEach(function (f) {
     _window[_namespace][f] = function () {
-      queue({f, a: arguments});
+      enqueue({f, a: arguments});
     };
   });
 
@@ -253,7 +279,7 @@ declare const __LOADER__IS_LAZY__: any;
 
   if (!lazy) {
     setTimeout(function () {
-      injectSdk(onLoadCallbacks);
+      injectCDNScriptTag();
     });
   }
 })(