diff --git a/src/main/views/webContentEvents.test.js b/src/main/views/webContentEvents.test.js index c59c9268..758e4a3c 100644 --- a/src/main/views/webContentEvents.test.js +++ b/src/main/views/webContentEvents.test.js @@ -5,6 +5,8 @@ import {shell, BrowserWindow} from 'electron'; +import {getLevel} from 'common/log'; + import ContextMenu from 'main/contextMenu'; import ViewManager from 'main/views/viewManager'; @@ -60,15 +62,13 @@ describe('main/views/webContentsEvents', () => { describe('willNavigate', () => { const webContentsEventManager = new WebContentsEventManager(); + webContentsEventManager.getServerURLFromWebContentsId = () => new URL('http://server-1.com'); const willNavigate = webContentsEventManager.generateWillNavigate(1); - - beforeEach(() => { - webContentsEventManager.getServerURLFromWebContentsId = jest.fn().mockImplementation(() => new URL('http://server-1.com')); - }); + const popupWindowSpy = jest.spyOn(webContentsEventManager, 'isTrustedPopupWindow'); afterEach(() => { - jest.resetAllMocks(); - jest.restoreAllMocks(); + event.preventDefault.mockClear(); + popupWindowSpy.mockReset(); webContentsEventManager.customLogins = {}; webContentsEventManager.popupWindow = undefined; }); @@ -84,8 +84,7 @@ describe('main/views/webContentsEvents', () => { }); it('should allow navigation when isTrustedPopup', () => { - const spy = jest.spyOn(webContentsEventManager, 'isTrustedPopupWindow'); - spy.mockReturnValue(true); + popupWindowSpy.mockReturnValue(true); willNavigate(event, 'http://externalurl.com/popup/subpath'); expect(event.preventDefault).not.toBeCalled(); }); @@ -131,7 +130,7 @@ describe('main/views/webContentsEvents', () => { }); afterEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); webContentsEventManager.customLogins = {}; }); @@ -171,7 +170,7 @@ describe('main/views/webContentsEvents', () => { afterEach(() => { webContentsEventManager.popupWindow = undefined; - jest.resetAllMocks(); + jest.clearAllMocks(); }); it('should deny on bad URL', () => { expect(newWindow({url: 'a-bad { expect(shell.openExternal).toBeCalledWith('https://google.com'); }); }); + + describe('consoleMessage', () => { + const webContentsEventManager = new WebContentsEventManager(); + const logObject = { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + verbose: jest.fn(), + withPrefix: jest.fn().mockReturnThis(), + }; + webContentsEventManager.log = jest.fn().mockReturnValue(logObject); + const consoleMessage = webContentsEventManager.generateHandleConsoleMessage(); + + afterEach(() => { + getLevel.mockReset(); + }); + + it('should respect logging levels', () => { + consoleMessage({}, 0, 'test0', 0, ''); + expect(logObject.verbose).toHaveBeenCalledWith('test0'); + + consoleMessage({}, 1, 'test1', 0, ''); + expect(logObject.info).toHaveBeenCalledWith('test1'); + + consoleMessage({}, 2, 'test2', 0, ''); + expect(logObject.warn).toHaveBeenCalledWith('test2'); + + consoleMessage({}, 3, 'test3', 0, ''); + expect(logObject.error).toHaveBeenCalledWith('test3'); + }); + + it('should only add line numbers for debug and silly', () => { + getLevel.mockReturnValue('debug'); + consoleMessage({}, 0, 'test1', 42, 'meaning_of_life.js'); + expect(logObject.verbose).toHaveBeenCalledWith('test1', '(meaning_of_life.js:42)'); + + getLevel.mockReturnValue('info'); + consoleMessage({}, 0, 'test2', 42, 'meaning_of_life.js'); + expect(logObject.verbose).not.toHaveBeenCalledWith('test2', '(meaning_of_life.js:42)'); + }); + }); }); diff --git a/src/main/views/webContentEvents.ts b/src/main/views/webContentEvents.ts index e6db3a65..8b478ec6 100644 --- a/src/main/views/webContentEvents.ts +++ b/src/main/views/webContentEvents.ts @@ -1,10 +1,12 @@ // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import path from 'path'; + import {BrowserWindow, session, shell, WebContents, Event} from 'electron'; import Config from 'common/config'; -import {Logger} from 'common/log'; +import {Logger, getLevel} from 'common/log'; import ServerManager from 'common/servers/serverManager'; import { isAdminUrl, @@ -40,6 +42,13 @@ type CustomLogin = { inProgress: boolean; } +enum ConsoleMessageLevel { + Verbose, + Info, + Warning, + Error +} + const log = new Logger('WebContentsEventManager'); const scheme = protocols && protocols[0] && protocols[0].schemes && protocols[0].schemes[0]; @@ -289,6 +298,30 @@ export class WebContentsEventManager { }; }; + private generateHandleConsoleMessage = (webContentsId: number) => (_: Event, level: number, message: string, line: number, sourceId: string) => { + const wcLog = this.log(webContentsId).withPrefix('renderer'); + let logFn = wcLog.verbose; + switch (level) { + case ConsoleMessageLevel.Error: + logFn = wcLog.error; + break; + case ConsoleMessageLevel.Warning: + logFn = wcLog.warn; + break; + case ConsoleMessageLevel.Info: + logFn = wcLog.info; + break; + } + + // Only include line entries if we're debugging + const entries = [message]; + if (['debug', 'silly'].includes(getLevel())) { + entries.push(`(${path.basename(sourceId)}:${line})`); + } + + logFn(...entries); + } + removeWebContentsListeners = (id: number) => { if (this.listeners[id]) { this.listeners[id](); @@ -324,12 +357,16 @@ export class WebContentsEventManager { const newWindow = this.generateNewWindowListener(contents.id, spellcheck); contents.setWindowOpenHandler(newWindow); + const consoleMessage = this.generateHandleConsoleMessage(contents.id); + contents.on('console-message', consoleMessage); + addListeners?.(contents); const removeWebContentsListeners = () => { try { contents.removeListener('will-navigate', willNavigate); contents.removeListener('did-start-navigation', didStartNavigation); + contents.removeListener('console-message', consoleMessage); removeListeners?.(contents); } catch (e) { this.log(contents.id).error(`Error while trying to detach listeners, this might be ok if the process crashed: ${e}`);