Skip to content

Improve logging error messages #1497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2022
Merged

Improve logging error messages #1497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions projects/angular-auth-oidc-client/src/lib/logging/logger.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,18 @@ describe('Logger Service', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should log error as default', () => {
it('should log error as default if error is string', () => {
const spy = spyOn(console, 'error');
loggerService.logError({ configId: 'configId1' }, 'some message');
expect(spy).toHaveBeenCalledOnceWith('[ERROR] configId1 - some message');
});

it('should log error as default if error is object', () => {
const spy = spyOn(console, 'error');
loggerService.logError({ configId: 'configId1' }, { some: 'message' });
expect(spy).toHaveBeenCalledOnceWith('[ERROR] configId1 - {"some":"message"}');
});

it('should always log error with args', () => {
const spy = spyOn(console, 'error');
loggerService.logError({ configId: 'configId1' }, 'some message', 'arg1', 'arg2');
Expand All @@ -50,6 +56,13 @@ describe('Logger Service', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should not log if no config is given', () => {
const spy = spyOn(console, 'warn');

loggerService.logWarning(null, 'some message');
expect(spy).not.toHaveBeenCalled();
});

it('should not log if no log level is set (undefined)', () => {
const spy = spyOn(console, 'warn');

Expand All @@ -65,13 +78,20 @@ describe('Logger Service', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should log warning when loglevel is Warn', () => {
it('should log warning when loglevel is Warn and message is string', () => {
const spy = spyOn(console, 'warn');

loggerService.logWarning({ configId: 'configId1', logLevel: LogLevel.Warn }, 'some message');
expect(spy).toHaveBeenCalledOnceWith('[WARN] configId1 - some message');
});

it('should log warning when loglevel is Warn and message is object', () => {
const spy = spyOn(console, 'warn');

loggerService.logWarning({ configId: 'configId1', logLevel: LogLevel.Warn }, { some: 'message' });
expect(spy).toHaveBeenCalledOnceWith('[WARN] configId1 - {"some":"message"}');
});

it('should log warning when loglevel is Warn with args', () => {
const spy = spyOn(console, 'warn');

Expand Down Expand Up @@ -116,13 +136,20 @@ describe('Logger Service', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should log when loglevel is Debug', () => {
it('should log when loglevel is Debug and value is string', () => {
const spy = spyOn(console, 'log');

loggerService.logDebug({ configId: 'configId1', logLevel: LogLevel.Debug }, 'some message');
expect(spy).toHaveBeenCalledOnceWith('[DEBUG] configId1 - some message');
});

it('should log when loglevel is Debug and value is object', () => {
const spy = spyOn(console, 'log');

loggerService.logDebug({ configId: 'configId1', logLevel: LogLevel.Debug }, { some: 'message' });
expect(spy).toHaveBeenCalledOnceWith('[DEBUG] configId1 - {"some":"message"}');
});

it('should log when loglevel is Debug with args', () => {
const spy = spyOn(console, 'log');

Expand Down
19 changes: 13 additions & 6 deletions projects/angular-auth-oidc-client/src/lib/logging/logger.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ export class LoggerService {
}

const { configId } = configuration;
const messageToLog = this.isObject(message) ? JSON.stringify(message) : message;

if (!!args && !!args.length) {
this.abstractLoggerService.logError(`[ERROR] ${configId} - ${message}`, ...args);
this.abstractLoggerService.logError(`[ERROR] ${configId} - ${messageToLog}`, ...args);
} else {
this.abstractLoggerService.logError(`[ERROR] ${configId} - ${message}`);
this.abstractLoggerService.logError(`[ERROR] ${configId} - ${messageToLog}`);
}
}

Expand All @@ -35,11 +36,12 @@ export class LoggerService {
}

const { configId } = configuration;
const messageToLog = this.isObject(message) ? JSON.stringify(message) : message;

if (!!args && !!args.length) {
this.abstractLoggerService.logWarning(`[WARN] ${configId} - ${message}`, ...args);
this.abstractLoggerService.logWarning(`[WARN] ${configId} - ${messageToLog}`, ...args);
} else {
this.abstractLoggerService.logWarning(`[WARN] ${configId} - ${message}`);
this.abstractLoggerService.logWarning(`[WARN] ${configId} - ${messageToLog}`);
}
}

Expand All @@ -57,11 +59,12 @@ export class LoggerService {
}

const { configId } = configuration;
const messageToLog = this.isObject(message) ? JSON.stringify(message) : message;

if (!!args && !!args.length) {
this.abstractLoggerService.logDebug(`[DEBUG] ${configId} - ${message}`, ...args);
this.abstractLoggerService.logDebug(`[DEBUG] ${configId} - ${messageToLog}`, ...args);
} else {
this.abstractLoggerService.logDebug(`[DEBUG] ${configId} - ${message}`);
this.abstractLoggerService.logDebug(`[DEBUG] ${configId} - ${messageToLog}`);
}
}

Expand Down Expand Up @@ -90,4 +93,8 @@ export class LoggerService {

return logLevel === LogLevel.None;
}

private isObject(possibleObject: any): boolean {
return Object.prototype.toString.call(possibleObject) === '[object Object]';
}
}