Skip to content

Conversation

@michaelwittwer
Copy link
Member

The ScriptLoaderService was logging script loading failures at ERROR level, which could trigger unnecessary alarms in monitoring systems when failures are expected (e.g., ad blockers preventing Google Analytics from loading).

Changes

Changed the log level from ERROR to WARN when scripts fail to load:

// Before
this.logger.error(err)

// After  
this.logger.warn(err)

This change allows consumer code to decide how to handle script loading failures. The service still:

  • Creates proper ScriptLoaderError objects with all details
  • Rejects the promise so consumers can catch and handle errors
  • Maintains all existing functionality

Example Impact

Consider loading Google Analytics with an ad blocker:

scriptLoaderService.addScriptToHead('https://www.google-analytics.com/analytics.js')
  .then(() => console.log('Analytics loaded'))
  .catch((error) => {
    // Consumer decides: is this an error or expected behavior?
    if (isAdBlockerActive()) {
      console.log('Tracking blocked - not an error');
    } else {
      console.error('Unexpected script loading error:', error);
    }
  });

Before: Service logs at ERROR level → monitoring alarms triggered
After: Service logs at WARN level → no false alarms, consumer controls error handling

Added comprehensive test coverage to verify the new logging behavior while ensuring all existing functionality remains intact.

Fixes #52.

… issues

let's let the consumer decide how to handle, since errors can be "expected"
 - @shiftcode/ngx-core@11.1.0-pr52.0
@michaelwittwer michaelwittwer marked this pull request as ready for review August 21, 2025 13:12
@michaelwittwer michaelwittwer merged commit bfebf9e into main Aug 21, 2025
2 checks passed
@michaelwittwer michaelwittwer deleted the #52-script-loader-service-warn-on-error branch August 21, 2025 14:01
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 this pull request may close these issues.

ScriptLoaderService should not log with level error if unable to load script

4 participants