Skip to content

Commit

Permalink
Merge pull request #223 from dbfannin/fix-circular
Browse files Browse the repository at this point in the history
avoids fail on complex structure when logging
  • Loading branch information
bmtheo authored Feb 23, 2021
2 parents 1d99652 + 09822aa commit d3e7075
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 10 deletions.
2 changes: 1 addition & 1 deletion projects/demo/src/app/app.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
<div class="controls">
<h3>Please open your console to see the output of the demo</h3>
<app-logger (logToConsole)="handleLog($event)"></app-logger>
<app-log-config (loggerLevelChange)="handleLogLevelChange($event)" (disableFileDetails)="handleDisableFileDetails($event)"></app-log-config>
<app-log-config (loggerLevelChange)="handleLogLevelChange($event)" (disableFileDetails)="handleDisableFileDetails($event)" (serverLogging)="serverLogging($event)"></app-log-config>
</div>
7 changes: 7 additions & 0 deletions projects/demo/src/app/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export class AppComponent {
handleLogLevelChange(newLevel: NgxLoggerLevel) {
const updatedConfig = this.logger.getConfigSnapshot();
updatedConfig.level = newLevel;
updatedConfig.serverLogLevel = newLevel;
this.logger.updateConfig(updatedConfig);
}

Expand Down Expand Up @@ -57,4 +58,10 @@ export class AppComponent {
updatedConfig.disableFileDetails = disableFileDetails;
this.logger.updateConfig(updatedConfig);
}

serverLogging(enabled: boolean) {
const updatedConfig = this.logger.getConfigSnapshot();
updatedConfig.serverLoggingUrl = enabled ? '/dummyURL' : null;
this.logger.updateConfig(updatedConfig);
}
}
3 changes: 2 additions & 1 deletion projects/demo/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
MatSlideToggleModule,
MatTooltipModule,
} from '@angular/material';
import { LoggerModule, NgxLoggerLevel } from 'ngx-logger';
import { LoggerModule, NGXLogger, NgxLoggerLevel } from 'ngx-logger';

import { AppComponent } from './app.component';
import { LogConfigComponent } from './log-config/log-config.component';
Expand All @@ -29,6 +29,7 @@ import { HttpClientModule } from '@angular/common/http';
ReactiveFormsModule,
LoggerModule.forRoot({
level: NgxLoggerLevel.DEBUG,
serverLogLevel: NgxLoggerLevel.DEBUG,
}),
MatButtonModule,
MatCardModule,
Expand Down
1 change: 1 addition & 0 deletions projects/demo/src/app/log-config/log-config.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ <h3>Logger Configuration</h3>
<hr/>

<mat-slide-toggle (change)="disableFileDetailsChange($event)" color="primary" matTooltip="The file name and line won't be logged">Disable file details</mat-slide-toggle>
<mat-slide-toggle class="server-logging" (change)="serverLoggingChange($event)" color="primary" matTooltip="The logger will send logs on a dummy url" >Server logging</mat-slide-toggle>

</mat-card-content>
</mat-card>
4 changes: 4 additions & 0 deletions projects/demo/src/app/log-config/log-config.component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ button {
display: flex;
justify-content: center;
}

.server-logging {
margin-left: 16px;
}
7 changes: 7 additions & 0 deletions projects/demo/src/app/log-config/log-config.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export class LogConfigComponent {
@Output()
disableFileDetails: EventEmitter<boolean> = new EventEmitter<boolean>();

@Output()
serverLogging: EventEmitter<boolean> = new EventEmitter<boolean>();

/**
* Get the chip color based on the current logger level configuration
*/
Expand Down Expand Up @@ -56,4 +59,8 @@ export class LogConfigComponent {
disableFileDetailsChange(change: MatSlideToggleChange) {
this.disableFileDetails.emit(change.checked);
}

serverLoggingChange(change: MatSlideToggleChange) {
this.serverLogging.emit(change.checked);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

</mat-card-content>
<mat-card-actions align="end">
<button mat-raised-button type="button" (click)="logComplex()">Log Complex Structure</button>
<button mat-raised-button type="submit" color="primary" [disabled]="!loggerForm.valid">Log Message</button>
</mat-card-actions>
</mat-card>
Expand Down
16 changes: 13 additions & 3 deletions projects/demo/src/app/logger-form/logger-form.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Component, OnInit, Output, EventEmitter} from '@angular/core';
import {Validators, FormBuilder} from '@angular/forms';
import {NgxLoggerLevel} from 'ngx-logger';
import {Validators, FormBuilder, FormGroup} from '@angular/forms';
import {NGXLogger, NgxLoggerLevel} from 'ngx-logger';

import {LogEvent} from '../models/log-event.model';

Expand Down Expand Up @@ -36,7 +36,10 @@ export class LoggerFormComponent implements OnInit {
{value: NgxLoggerLevel.ERROR, viewValue: 'Error'}
];

constructor(private fb: FormBuilder) {
constructor(
private fb: FormBuilder,
private logger: NGXLogger
) {
}

ngOnInit() {
Expand All @@ -48,4 +51,11 @@ export class LoggerFormComponent implements OnInit {
handleFormSubmission() {
this.logToConsole.emit(this.loggerForm.value);
}

logComplex() {
const complexStructure = new FormGroup({ sub: new FormGroup({}) });
this.logger.error('Test complex', complexStructure);
this.logger.error(complexStructure);
}

}
20 changes: 20 additions & 0 deletions src/lib/logger.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { NgxLoggerLevel } from './types/logger-level.enum';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { LogPosition } from 'dist/ngx-logger/lib/types/log-position';
import { of } from 'rxjs';
import { FormGroup } from '@angular/forms';

describe('NGXLogger', () => {
beforeEach(() => {
Expand Down Expand Up @@ -45,6 +46,25 @@ describe('NGXLogger', () => {
}
));

it('should handle complex circular structures', inject(
[NGXLogger],
(logger: NGXLogger) => {
// This structure is not "stringifyable" this make sure anything can be logged
// Before that we used JSON.stringify and it was not working
const complexStructure = new FormGroup({ sub: new FormGroup({}) });

spyOn(console, 'error');

logger.error('error', complexStructure);

expect(console.error).toHaveBeenCalledWith(jasmine.anything(), jasmine.anything(), jasmine.anything(), complexStructure);

logger.error(complexStructure);

expect(console.error).toHaveBeenCalledWith(jasmine.anything(), jasmine.anything(), complexStructure);
}
));

describe('trace', () => {
it('should call _log with trace', inject(
[NGXLogger],
Expand Down
12 changes: 7 additions & 5 deletions src/lib/logger.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ export class NGXLogger {
const logLevelString = Levels[level];

message = typeof message === 'function' ? message() : message;
message = NGXLoggerUtils.prepareMessage(message);

// only use validated parameters for HTTP requests
const validatedAdditionalParameters = NGXLoggerUtils.prepareAdditionalParameters(additional);
Expand All @@ -187,7 +186,10 @@ export class NGXLogger {

this.mapperService.getCallerDetails(config.enableSourceMaps, config.proxiedSteps).subscribe((callerDetails: LogPosition) => {
const logObject: NGXLogInterface = {
message: message,
// prepareMessage is needed to match NGXLogInterface
// Even though I think message should be of type any (same as console.xxx signature)
// I'm not doing this right now as this would be a breaking change
message: NGXLoggerUtils.prepareMessage(message),
additional: validatedAdditionalParameters,
level: level,
timestamp: timestamp,
Expand All @@ -200,9 +202,9 @@ export class NGXLogger {
}

if (isLog2Server) {
// make sure the stack gets sent to the server
message = message instanceof Error ? message.stack : message;
logObject.message = message;
// make sure the stack gets sent to the server (without altering the message for console logging)
logObject.message = message instanceof Error ? message.stack : message;
logObject.message = NGXLoggerUtils.prepareMessage(logObject.message);

const headers = this._customHttpHeaders || new HttpHeaders();
headers.set('Content-Type', 'application/json');
Expand Down

0 comments on commit d3e7075

Please sign in to comment.