Skip to content

Commit 75686e8

Browse files
jermoweryjelbourn
jermowery
authored and
jelbourn
committed
fix(icon): use ErrorLogger to log http errors (#16855)
1 parent 66593f4 commit 75686e8

File tree

3 files changed

+63
-26
lines changed

3 files changed

+63
-26
lines changed

‎src/material/icon/icon-registry.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {DOCUMENT} from '@angular/common';
1010
import {HttpClient, HttpErrorResponse} from '@angular/common/http';
1111
import {
12+
ErrorHandler,
1213
Inject,
1314
Injectable,
1415
InjectionToken,
@@ -132,7 +133,9 @@ export class MatIconRegistry implements OnDestroy {
132133
constructor(
133134
@Optional() private _httpClient: HttpClient,
134135
private _sanitizer: DomSanitizer,
135-
@Optional() @Inject(DOCUMENT) document: any) {
136+
@Optional() @Inject(DOCUMENT) document: any,
137+
// @breaking-change 9.0.0 _errorHandler parameter to be made required
138+
@Optional() private readonly _errorHandler?: ErrorHandler) {
136139
this._document = document;
137140
}
138141

@@ -373,7 +376,13 @@ export class MatIconRegistry implements OnDestroy {
373376

374377
// Swallow errors fetching individual URLs so the
375378
// combined Observable won't necessarily fail.
376-
console.error(`Loading icon set URL: ${url} failed: ${err.message}`);
379+
const errorMessage = `Loading icon set URL: ${url} failed: ${err.message}`;
380+
// @breaking-change 9.0.0 _errorHandler parameter to be made required
381+
if (this._errorHandler) {
382+
this._errorHandler.handleError(new Error(errorMessage));
383+
} else {
384+
console.error(errorMessage);
385+
}
377386
return observableOf(null);
378387
})
379388
);
@@ -515,7 +524,7 @@ export class MatIconRegistry implements OnDestroy {
515524
* Converts an element into an SVG node by cloning all of its children.
516525
*/
517526
private _toSvgElement(element: Element): SVGElement {
518-
let svg = this._svgElementFromString('<svg></svg>');
527+
const svg = this._svgElementFromString('<svg></svg>');
519528

520529
for (let i = 0; i < element.childNodes.length; i++) {
521530
if (element.childNodes[i].nodeType === this._document.ELEMENT_NODE) {
@@ -616,8 +625,9 @@ export function ICON_REGISTRY_PROVIDER_FACTORY(
616625
parentRegistry: MatIconRegistry,
617626
httpClient: HttpClient,
618627
sanitizer: DomSanitizer,
619-
document?: any) {
620-
return parentRegistry || new MatIconRegistry(httpClient, sanitizer, document);
628+
document?: any,
629+
errorHandler?: ErrorHandler) {
630+
return parentRegistry || new MatIconRegistry(httpClient, sanitizer, document, errorHandler);
621631
}
622632

623633
/** @docs-private */
@@ -628,6 +638,7 @@ export const ICON_REGISTRY_PROVIDER = {
628638
[new Optional(), new SkipSelf(), MatIconRegistry],
629639
[new Optional(), HttpClient],
630640
DomSanitizer,
641+
[new Optional(), ErrorHandler],
631642
[new Optional(), DOCUMENT as InjectionToken<any>],
632643
],
633644
useFactory: ICON_REGISTRY_PROVIDER_FACTORY,

‎src/material/icon/icon.spec.ts

Lines changed: 45 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {inject, async, fakeAsync, tick, TestBed} from '@angular/core/testing';
22
import {SafeResourceUrl, DomSanitizer, SafeHtml} from '@angular/platform-browser';
33
import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing';
4-
import {Component} from '@angular/core';
4+
import {Component, ErrorHandler} from '@angular/core';
55
import {MatIconModule, MAT_ICON_LOCATION} from './index';
66
import {MatIconRegistry, getMatIconNoHttpProviderError} from './icon-registry';
77
import {FAKE_SVGS} from './fake-svgs';
@@ -40,11 +40,13 @@ function verifyPathChildElement(element: Element, attributeValue: string): void
4040

4141
describe('MatIcon', () => {
4242
let fakePath: string;
43+
let errorHandler: jasmine.SpyObj<ErrorHandler>;
4344

4445
beforeEach(async(() => {
4546
// The $ prefix tells Karma not to try to process the
4647
// request so that we don't get warnings in our logs.
4748
fakePath = '/$fake-path';
49+
errorHandler = jasmine.createSpyObj('errorHandler', ['handleError']);
4850

4951
TestBed.configureTestingModule({
5052
imports: [HttpClientTestingModule, MatIconModule],
@@ -59,10 +61,16 @@ describe('MatIcon', () => {
5961
SvgIconWithUserContent,
6062
IconWithLigatureAndSvgBinding,
6163
],
62-
providers: [{
63-
provide: MAT_ICON_LOCATION,
64-
useValue: {getPathname: () => fakePath}
65-
}]
64+
providers: [
65+
{
66+
provide: MAT_ICON_LOCATION,
67+
useValue: {getPathname: () => fakePath}
68+
},
69+
{
70+
provide: ErrorHandler,
71+
useValue: errorHandler,
72+
},
73+
]
6674
});
6775

6876
TestBed.compileComponents();
@@ -80,15 +88,15 @@ describe('MatIcon', () => {
8088
}));
8189

8290
it('should include notranslate class by default', () => {
83-
let fixture = TestBed.createComponent(IconWithColor);
91+
const fixture = TestBed.createComponent(IconWithColor);
8492

8593
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
8694
expect(matIconElement.classList.contains('notranslate'))
8795
.toBeTruthy('Expected the mat-icon element to include the notranslate class');
8896
});
8997

9098
it('should apply class based on color attribute', () => {
91-
let fixture = TestBed.createComponent(IconWithColor);
99+
const fixture = TestBed.createComponent(IconWithColor);
92100

93101
const testComponent = fixture.componentInstance;
94102
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -100,7 +108,7 @@ describe('MatIcon', () => {
100108
});
101109

102110
it('should apply a class if there is no color', () => {
103-
let fixture = TestBed.createComponent(IconWithColor);
111+
const fixture = TestBed.createComponent(IconWithColor);
104112

105113
const testComponent = fixture.componentInstance;
106114
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -140,7 +148,7 @@ describe('MatIcon', () => {
140148

141149
describe('Ligature icons', () => {
142150
it('should add material-icons class by default', () => {
143-
let fixture = TestBed.createComponent(IconWithLigature);
151+
const fixture = TestBed.createComponent(IconWithLigature);
144152

145153
const testComponent = fixture.componentInstance;
146154
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -153,7 +161,7 @@ describe('MatIcon', () => {
153161
it('should use alternate icon font if set', () => {
154162
iconRegistry.setDefaultFontSetClass('myfont');
155163

156-
let fixture = TestBed.createComponent(IconWithLigature);
164+
const fixture = TestBed.createComponent(IconWithLigature);
157165

158166
const testComponent = fixture.componentInstance;
159167
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -165,7 +173,7 @@ describe('MatIcon', () => {
165173

166174
it('should not clear the text of a ligature icon if the svgIcon is bound to something falsy',
167175
() => {
168-
let fixture = TestBed.createComponent(IconWithLigatureAndSvgBinding);
176+
const fixture = TestBed.createComponent(IconWithLigatureAndSvgBinding);
169177

170178
const testComponent = fixture.componentInstance;
171179
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -182,7 +190,7 @@ describe('MatIcon', () => {
182190
iconRegistry.addSvgIcon('fluffy', trustUrl('cat.svg'));
183191
iconRegistry.addSvgIcon('fido', trustUrl('dog.svg'));
184192

185-
let fixture = TestBed.createComponent(IconFromSvgName);
193+
const fixture = TestBed.createComponent(IconFromSvgName);
186194
let svgElement: SVGElement;
187195
const testComponent = fixture.componentInstance;
188196
const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -219,7 +227,7 @@ describe('MatIcon', () => {
219227
iconRegistry.addSvgIcon('fluffy', trustUrl('cat.svg'), {viewBox: '0 0 27 27'});
220228
iconRegistry.addSvgIcon('fido', trustUrl('dog.svg'), {viewBox: '0 0 43 43'});
221229

222-
let fixture = TestBed.createComponent(IconFromSvgName);
230+
const fixture = TestBed.createComponent(IconFromSvgName);
223231
let svgElement: SVGElement;
224232
const testComponent = fixture.componentInstance;
225233
const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -242,7 +250,7 @@ describe('MatIcon', () => {
242250
iconRegistry.addSvgIcon('fluffy', 'farm-set-1.svg');
243251

244252
expect(() => {
245-
let fixture = TestBed.createComponent(IconFromSvgName);
253+
const fixture = TestBed.createComponent(IconFromSvgName);
246254
fixture.componentInstance.iconName = 'fluffy';
247255
fixture.detectChanges();
248256
}).toThrowError(/unsafe value used in a resource URL context/);
@@ -252,12 +260,30 @@ describe('MatIcon', () => {
252260
iconRegistry.addSvgIconSetInNamespace('farm', 'farm-set-1.svg');
253261

254262
expect(() => {
255-
let fixture = TestBed.createComponent(IconFromSvgName);
263+
const fixture = TestBed.createComponent(IconFromSvgName);
256264
fixture.componentInstance.iconName = 'farm:pig';
257265
fixture.detectChanges();
258266
}).toThrowError(/unsafe value used in a resource URL context/);
259267
});
260268

269+
270+
it('should delegate http error logging to the ErrorHandler', () => {
271+
iconRegistry.addSvgIconSetInNamespace('farm', trustUrl('farm-set-1.svg'));
272+
273+
const fixture = TestBed.createComponent(IconFromSvgName);
274+
const testComponent = fixture.componentInstance;
275+
276+
testComponent.iconName = 'farm:pig';
277+
fixture.detectChanges();
278+
http.expectOne('farm-set-1.svg').error(new ErrorEvent('Network error'));
279+
fixture.detectChanges();
280+
281+
expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
282+
expect(errorHandler.handleError.calls.argsFor(0)[0].message).toEqual(
283+
'Loading icon set URL: farm-set-1.svg failed: Http failure response ' +
284+
'for farm-set-1.svg: 0 ');
285+
});
286+
261287
it('should extract icon from SVG icon set', () => {
262288
iconRegistry.addSvgIconSetInNamespace('farm', trustUrl('farm-set-1.svg'));
263289

@@ -491,7 +517,7 @@ describe('MatIcon', () => {
491517
it('should remove the SVG element from the DOM when the binding is cleared', () => {
492518
iconRegistry.addSvgIconSet(trustUrl('arrow-set.svg'));
493519

494-
let fixture = TestBed.createComponent(IconFromSvgName);
520+
const fixture = TestBed.createComponent(IconFromSvgName);
495521

496522
const testComponent = fixture.componentInstance;
497523
const icon = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -534,7 +560,7 @@ describe('MatIcon', () => {
534560
iconRegistry.addSvgIconLiteral('fluffy', trustHtml(FAKE_SVGS.cat));
535561
iconRegistry.addSvgIconLiteral('fido', trustHtml(FAKE_SVGS.dog));
536562

537-
let fixture = TestBed.createComponent(IconFromSvgName);
563+
const fixture = TestBed.createComponent(IconFromSvgName);
538564
let svgElement: SVGElement;
539565
const testComponent = fixture.componentInstance;
540566
const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -561,7 +587,7 @@ describe('MatIcon', () => {
561587
iconRegistry.addSvgIconLiteral('fluffy', trustHtml(FAKE_SVGS.cat), {viewBox: '0 0 43 43'});
562588
iconRegistry.addSvgIconLiteral('fido', trustHtml(FAKE_SVGS.dog), {viewBox: '0 0 27 27'});
563589

564-
let fixture = TestBed.createComponent(IconFromSvgName);
590+
const fixture = TestBed.createComponent(IconFromSvgName);
565591
let svgElement: SVGElement;
566592
const testComponent = fixture.componentInstance;
567593
const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
@@ -916,7 +942,7 @@ describe('MatIcon without HttpClientModule', () => {
916942
expect(() => {
917943
iconRegistry.addSvgIcon('fido', sanitizer.bypassSecurityTrustResourceUrl('dog.svg'));
918944

919-
let fixture = TestBed.createComponent(IconFromSvgName);
945+
const fixture = TestBed.createComponent(IconFromSvgName);
920946

921947
fixture.componentInstance.iconName = 'fido';
922948
fixture.detectChanges();

‎tools/public_api_guard/material/icon.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export declare const ICON_REGISTRY_PROVIDER: {
1212
useFactory: typeof ICON_REGISTRY_PROVIDER_FACTORY;
1313
};
1414

15-
export declare function ICON_REGISTRY_PROVIDER_FACTORY(parentRegistry: MatIconRegistry, httpClient: HttpClient, sanitizer: DomSanitizer, document?: any): MatIconRegistry;
15+
export declare function ICON_REGISTRY_PROVIDER_FACTORY(parentRegistry: MatIconRegistry, httpClient: HttpClient, sanitizer: DomSanitizer, document?: any, errorHandler?: ErrorHandler): MatIconRegistry;
1616

1717
export interface IconOptions {
1818
viewBox?: string;
@@ -43,7 +43,7 @@ export declare class MatIconModule {
4343
}
4444

4545
export declare class MatIconRegistry implements OnDestroy {
46-
constructor(_httpClient: HttpClient, _sanitizer: DomSanitizer, document: any);
46+
constructor(_httpClient: HttpClient, _sanitizer: DomSanitizer, document: any, _errorHandler?: ErrorHandler | undefined);
4747
addSvgIcon(iconName: string, url: SafeResourceUrl, options?: IconOptions): this;
4848
addSvgIconInNamespace(namespace: string, iconName: string, url: SafeResourceUrl, options?: IconOptions): this;
4949
addSvgIconLiteral(iconName: string, literal: SafeHtml, options?: IconOptions): this;

0 commit comments

Comments
 (0)