Merge pull request #444 from nextcloud/feat/errors

Allow logging errors when passed as message parameter

lib/ConsoleLogger.ts

@@ -5,20 +5,33 @@ export class ConsoleLogger implements ILogger {
     private context: any
 
     constructor(context: any) {
-        this.context = context
+        this.context = context || {}
     }
 
-    private formatMessage(message: string, level: LogLevel, context: any): string {
+    private formatMessage(message: string|Error, level: LogLevel, context: any): string {
         let msg = '[' + LogLevel[level].toUpperCase() + ']'
+
         if (context && context.app) {
             msg += ' ' + context.app + ': '
         }
-        return msg + message
+
+        if (typeof message === 'string') return msg + message
+
+        // basic error formatting
+        msg += `Unexpected ${message.name}`
+        if (message.message) msg += ` "${message.message}"`
+        // only add stack trace when debugging
+        if (level === LogLevel.Debug && message.stack) msg += `\n\nStack trace:\n${message.stack}`
+
+        return msg
     }
 
-    log(level: LogLevel, message: string, context: object) {
+    log(level: LogLevel, message: string|Error, context: object) {
         if (level < this.context?.level) return;
 
+        // Add error object to context
+        if (typeof message === 'object' && (context as any).error === undefined) (context as any).error = message
+
         switch (level) {
             case LogLevel.Debug:
                 console.debug(this.formatMessage(message, LogLevel.Debug, context), context)
@@ -39,23 +52,23 @@ export class ConsoleLogger implements ILogger {
         }
     }
 
-    debug(message: string, context?: object): void {
+    debug(message: string|Error, context?: object): void {
         this.log(LogLevel.Debug, message, Object.assign({}, this.context, context))
     }
 
-    info(message: string, context?: object): void {
+    info(message: string|Error, context?: object): void {
         this.log(LogLevel.Info, message, Object.assign({}, this.context, context))
     }
 
-    warn(message: string, context?: object): void {
+    warn(message: string|Error, context?: object): void {
         this.log(LogLevel.Warn, message, Object.assign({}, this.context, context))
     }
 
-    error(message: string, context?: object): void {
+    error(message: string|Error, context?: object): void {
         this.log(LogLevel.Error, message, Object.assign({}, this.context, context))
     }
 
-    fatal(message: string, context?: object): void {
+    fatal(message: string|Error, context?: object): void {
         this.log(LogLevel.Fatal, message, Object.assign({}, this.context, context))
     }
 

lib/contracts.ts

@@ -8,11 +8,11 @@ export enum LogLevel {
 
 export interface ILogger {
 
-    debug(message: string, context?: object): void
-    info(message: string, context?: object): void
-    warn(message: string, context?: object): void
-    error(message: string, context?: object): void
-    fatal(message: string, context?: object): void
+    debug(message: string|Error, context?: object): void
+    info(message: string|Error, context?: object): void
+    warn(message: string|Error, context?: object): void
+    error(message: string|Error, context?: object): void
+    fatal(message: string|Error, context?: object): void
 
 }
 

tests/ConsoleLogger.spec.ts

@@ -0,0 +1,73 @@
+import { ConsoleLogger, buildConsoleLogger } from '../lib/ConsoleLogger'
+
+// Dummy Error
+class MyError extends Error {
+    constructor(msg: string) {
+        super(msg)
+        this.name = 'MyError'
+    }
+}
+
+afterEach(() => {
+    jest.resetAllMocks()
+})
+
+test('building the console logger', () => {
+    const logger = buildConsoleLogger({ app: 'myapp' })
+    expect(logger).toBeInstanceOf(ConsoleLogger)
+
+    // ensure initial context is preserved
+    const consoleArgs = [] as any[]
+    const warn = jest.spyOn(console, "warn").mockImplementation((...args) => consoleArgs.push(...args))
+    logger.warn('some message', { foo: 'bar' })
+    expect(warn).toHaveBeenCalledTimes(1)
+    expect(consoleArgs).toHaveLength(2)
+    expect(consoleArgs[1]).toHaveProperty('app', 'myapp')
+})
+
+describe('ConsoleLogger', () => {
+    afterEach(() => jest.resetAllMocks())
+
+    it('logs errors', () => {
+        const error = new MyError('some message')
+        const logger = new ConsoleLogger({})
+
+        const console = [] as any[][]
+        const warn = jest.spyOn(window.console, "warn").mockImplementation((msg, ctx) => console.push([msg, ctx]))
+
+        logger.warn(error)
+        expect(warn).toHaveBeenCalledTimes(1)
+        expect(console[0][0]).toContain('MyError')
+        expect(console[0][0]).toContain('some message')
+        expect(console[0][0]).not.toContain('Stack trace')
+        expect(console[0][1]).toHaveProperty('error', error)
+    })
+
+    it('logs error and stack trace on debug', () => {
+        const error = new MyError('some message')
+        const logger = new ConsoleLogger({})
+
+        const console = [] as any[][]
+        const debug = jest.spyOn(window.console, "debug").mockImplementation((msg, ctx) => console.push([msg, ctx]))
+
+        logger.debug(error)
+        expect(debug).toHaveBeenCalledTimes(1)
+        expect(console[0][0]).toContain('MyError')
+        expect(console[0][0]).toContain('some message')
+        expect(console[0][0]).toContain('Stack trace:')
+    })
+
+    it('logs error and does not override context', () => {
+        const error = new MyError('some message')
+        const logger = new ConsoleLogger({ error: 'none' })
+
+        const console = [] as any[][]
+        const warn = jest.spyOn(window.console, "warn").mockImplementation((msg, ctx) => console.push([msg, ctx]))
+
+        logger.warn(error)
+        expect(warn).toHaveBeenCalledTimes(1)
+        expect(console[0][0]).toContain('MyError')
+        expect(console[0][0]).toContain('some message')
+        expect(console[0][1]).toHaveProperty('error', 'none')
+    })
+})