From bcdbd58f06664b8cb3b532e0458813ecacc0667f Mon Sep 17 00:00:00 2001 From: Adam Weeden Date: Fri, 30 Apr 2021 23:21:37 -0400 Subject: [PATCH] App: replace console.xyz calls with loglevel.xyz, with a level controlled by app argv --verbose (#1172) In reference to request in https://github.com/nativefier/nativefier/pull/1168/files#r623753290 , this PR fixes a lot of the disparity in logging in the app, and fleshes the logging out a bit. --- .eslintrc.js | 1 + app/.eslintrc.js | 1 + app/package.json | 1 + app/src/components/mainWindow.ts | 26 +++++++++++++++++++----- app/src/components/menu.ts | 6 ++---- app/src/components/trayIcon.ts | 7 ++++++- app/src/helpers/helpers.ts | 7 ++++--- app/src/helpers/inferFlash.ts | 3 ++- app/src/main.ts | 34 +++++++++++++++++++++----------- app/src/preload.ts | 10 ++++++---- package.json | 2 +- src/build/prepareElectronApp.ts | 2 +- src/cli.ts | 2 +- 13 files changed, 69 insertions(+), 33 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 49400d2..928e6ad 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -15,6 +15,7 @@ module.exports = { 'plugin:@typescript-eslint/recommended-requiring-type-checking', ], rules: { + 'no-console': 'error', 'prettier/prettier': 'error', // TODO remove when done killing `any`s and making tsc strict '@typescript-eslint/ban-ts-comment': 'off', diff --git a/app/.eslintrc.js b/app/.eslintrc.js index 5ea6a39..a26872f 100644 --- a/app/.eslintrc.js +++ b/app/.eslintrc.js @@ -14,6 +14,7 @@ module.exports = { 'plugin:@typescript-eslint/recommended-requiring-type-checking', ], rules: { + 'no-console': 'error', 'prettier/prettier': 'error', // TODO remove when done killing `any`s and making tsc strict '@typescript-eslint/ban-ts-comment': 'off', diff --git a/app/package.json b/app/package.json index 0cc0b5d..1f33f42 100644 --- a/app/package.json +++ b/app/package.json @@ -16,6 +16,7 @@ "electron-dl": "^3.2.0", "electron-squirrel-startup": "^1.0.0", "electron-window-state": "^5.0.3", + "loglevel": "^1.7.1", "source-map-support": "^0.5.19" }, "devDependencies": { diff --git a/app/src/components/mainWindow.ts b/app/src/components/mainWindow.ts index 2ea59d7..c60bf3b 100644 --- a/app/src/components/mainWindow.ts +++ b/app/src/components/mainWindow.ts @@ -67,6 +67,7 @@ function injectCss(browserWindow: BrowserWindow): void { const cssToInject = getCssToInject(); browserWindow.webContents.on('did-navigate', () => { + log.debug('browserWindow.webContents.did-navigate'); // We must inject css early enough; so onHeadersReceived is a good place. // Will run multiple times, see `did-finish-load` below that unsets this handler. browserWindow.webContents.session.webRequest.onHeadersReceived( @@ -100,7 +101,7 @@ export function saveAppArgs(newAppArgs: any) { fs.writeFileSync(APP_ARGS_FILE_PATH, JSON.stringify(newAppArgs)); } catch (err) { // eslint-disable-next-line no-console - console.log( + log.warn( `WARNING: Ignored nativefier.json rewrital (${(err as Error).toString()})`, ); } @@ -189,18 +190,21 @@ export function createMainWindow( }; const onZoomIn = (): void => { + log.debug('onZoomIn'); withFocusedWindow((focusedWindow: BrowserWindow) => adjustWindowZoom(focusedWindow, ZOOM_INTERVAL), ); }; const onZoomOut = (): void => { + log.debug('onZoomOut'); withFocusedWindow((focusedWindow: BrowserWindow) => adjustWindowZoom(focusedWindow, -ZOOM_INTERVAL), ); }; const onZoomReset = (): void => { + log.debug('onZoomReset'); withFocusedWindow((focusedWindow: BrowserWindow) => { focusedWindow.webContents.zoomFactor = options.zoom; }); @@ -223,12 +227,14 @@ export function createMainWindow( }; const onGoBack = (): void => { + log.debug('onGoBack'); withFocusedWindow((focusedWindow) => { focusedWindow.webContents.goBack(); }); }; const onGoForward = (): void => { + log.debug('onGoForward'); withFocusedWindow((focusedWindow) => { focusedWindow.webContents.goForward(); }); @@ -241,6 +247,7 @@ export function createMainWindow( withFocusedWindow((focusedWindow) => void focusedWindow.loadURL(url)); const onBlockedExternalUrl = (url: string) => { + log.debug('onBlockedExternalUrl', url); // eslint-disable-next-line @typescript-eslint/no-floating-promises dialog.showMessageBox(mainWindow, { message: `Cannot navigate to external URL: ${url}`, @@ -250,6 +257,7 @@ export function createMainWindow( }; const onWillNavigate = (event: Event, urlToGo: string): void => { + log.debug('onWillNavigate', { event, urlToGo }); if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls)) { event.preventDefault(); if (options.blockExternalUrls) { @@ -302,6 +310,7 @@ export function createMainWindow( }; const createNewTab = (url: string, foreground: boolean): BrowserWindow => { + log.debug('createNewTab', { url, foreground }); withFocusedWindow((focusedWindow) => { const newTab = createNewWindow(url); focusedWindow.addTabbedWindow(newTab); @@ -332,6 +341,7 @@ export function createMainWindow( frameName: string, disposition, ): void => { + log.debug('onNewWindow', { event, urlToGo, frameName, disposition }); const preventDefault = (newGuest: any): void => { event.preventDefault(); if (newGuest) { @@ -355,6 +365,7 @@ export function createMainWindow( const sendParamsOnDidFinishLoad = (window: BrowserWindow): void => { window.webContents.on('did-finish-load', () => { + log.debug('sendParamsOnDidFinishLoad.window.webContents.did-finish-load'); // In children windows too: Restore pinch-to-zoom, disabled by default in recent Electron. // See https://github.com/nativefier/nativefier/issues/379#issuecomment-598612128 // and https://github.com/electron/electron/pull/12679 @@ -400,7 +411,8 @@ export function createMainWindow( sendParamsOnDidFinishLoad(mainWindow); if (options.counter) { - mainWindow.on('page-title-updated', (e, title) => { + mainWindow.on('page-title-updated', (event, title) => { + log.debug('mainWindow.page-title-updated', { event, title }); const counterValue = getCounterValue(title); if (counterValue) { setDockBadge(counterValue, options.bounce); @@ -410,17 +422,20 @@ export function createMainWindow( }); } else { ipcMain.on('notification', () => { + log.debug('ipcMain.notification'); if (!isOSX() || mainWindow.isFocused()) { return; } setDockBadge('•', options.bounce); }); mainWindow.on('focus', () => { + log.debug('mainWindow.focus'); setDockBadge(''); }); } ipcMain.on('notification-click', () => { + log.debug('ipcMain.notification-click'); mainWindow.show(); }); @@ -428,8 +443,7 @@ export function createMainWindow( ipcMain.on( 'session-interaction', (event, request: SessionInteractionRequest) => { - log.debug('session-interaction:event', event); - log.debug('session-interaction:request', request); + log.debug('ipcMain.session-interaction', { event, request }); const result: SessionInteractionResult = { id: request.id }; let awaitingPromise = false; @@ -457,7 +471,7 @@ export function createMainWindow( // This is a promise. We'll resolve it here otherwise it will blow up trying to serialize it in the reply result.value.then((trueResultValue) => { result.value = trueResultValue; - log.debug('session-interaction:result', result); + log.debug('ipcMain.session-interaction:result', result); event.reply('session-interaction-reply', result); }); awaitingPromise = true; @@ -496,6 +510,7 @@ export function createMainWindow( mainWindow.webContents.on('will-navigate', onWillNavigate); mainWindow.webContents.on('will-prevent-unload', onWillPreventUnload); mainWindow.webContents.on('did-finish-load', () => { + log.debug('mainWindow.webContents.did-finish-load'); // Restore pinch-to-zoom, disabled by default in recent Electron. // See https://github.com/nativefier/nativefier/issues/379#issuecomment-598309817 // and https://github.com/electron/electron/pull/12679 @@ -518,6 +533,7 @@ export function createMainWindow( mainWindow.on('new-tab', () => createNewTab(options.targetUrl, true)); mainWindow.on('close', (event) => { + log.debug('mainWindow.close', event); if (mainWindow.isFullScreen()) { if (nativeTabsSupported()) { mainWindow.moveTabToNewWindow(); diff --git a/app/src/components/menu.ts b/app/src/components/menu.ts index 9f27902..d8db8fb 100644 --- a/app/src/components/menu.ts +++ b/app/src/components/menu.ts @@ -2,6 +2,7 @@ import * as fs from 'fs'; import path from 'path'; import { Menu, clipboard, shell, MenuItemConstructorOptions } from 'electron'; +import * as log from 'loglevel'; type BookmarksLink = { type: 'link'; @@ -351,10 +352,7 @@ export function createMenu({ menuTemplate.splice(menuTemplate.length - 2, 0, bookmarksMenu); } } catch (err) { - console.warn( - 'Failed to load & parse bookmarks configuration JSON file.', - err, - ); + log.error('Failed to load & parse bookmarks configuration JSON file.', err); } const menu = Menu.buildFromTemplate(menuTemplate); diff --git a/app/src/components/trayIcon.ts b/app/src/components/trayIcon.ts index 6e633ea..6d4b80b 100644 --- a/app/src/components/trayIcon.ts +++ b/app/src/components/trayIcon.ts @@ -1,4 +1,5 @@ import { app, Tray, Menu, ipcMain, nativeImage, BrowserWindow } from 'electron'; +import log from 'loglevel'; import { getAppIcon, getCounterValue, isOSX } from '../helpers/helpers'; @@ -23,6 +24,7 @@ export function createTrayIcon( } const onClick = () => { + log.debug('onClick'); if (mainWindow.isVisible()) { mainWindow.hide(); } else { @@ -44,7 +46,8 @@ export function createTrayIcon( appIcon.on('click', onClick); if (options.counter) { - mainWindow.on('page-title-updated', (e, title) => { + mainWindow.on('page-title-updated', (event, title) => { + log.debug('mainWindow.page-title-updated', { event, title }); const counterValue = getCounterValue(title); if (counterValue) { appIcon.setToolTip(`(${counterValue}) ${options.name}`); @@ -54,6 +57,7 @@ export function createTrayIcon( }); } else { ipcMain.on('notification', () => { + log.debug('ipcMain.notification'); if (mainWindow.isFocused()) { return; } @@ -61,6 +65,7 @@ export function createTrayIcon( }); mainWindow.on('focus', () => { + log.debug('mainWindow.focus'); appIcon.setToolTip(options.name); }); } diff --git a/app/src/helpers/helpers.ts b/app/src/helpers/helpers.ts index 43cd55f..7948f60 100644 --- a/app/src/helpers/helpers.ts +++ b/app/src/helpers/helpers.ts @@ -3,6 +3,7 @@ import * as os from 'os'; import * as path from 'path'; import { BrowserWindow } from 'electron'; +import * as log from 'loglevel'; export const INJECT_DIR = path.join(__dirname, '..', 'inject'); @@ -67,7 +68,7 @@ export function linkIsInternal( : [newDomain, currentDomain]; return longerDomain.endsWith(shorterDomain); } catch (err) { - console.warn( + log.error( 'Failed to parse domains as determining if link is internal. From:', currentUrl, 'To:', @@ -97,7 +98,7 @@ export function getCssToInject(): string { path.resolve(path.join(INJECT_DIR, cssFileStat.name)), ); for (const cssFile of cssFiles) { - console.log(`Injecting CSS file: ${cssFile}`); + log.debug('Injecting CSS file', cssFile); const cssFileData = fs.readFileSync(cssFile); cssToInject += `/* ${cssFile} */\n\n ${cssFileData}\n\n`; } @@ -111,7 +112,7 @@ export function debugLog(browserWindow: BrowserWindow, message: string): void { setTimeout(() => { browserWindow.webContents.send('debug', message); }, 3000); - console.info(message); + log.info(message); } export function getAppIcon(): string { diff --git a/app/src/helpers/inferFlash.ts b/app/src/helpers/inferFlash.ts index bd9f189..1603f8a 100644 --- a/app/src/helpers/inferFlash.ts +++ b/app/src/helpers/inferFlash.ts @@ -1,4 +1,5 @@ import * as fs from 'fs'; +import log from 'loglevel'; import * as path from 'path'; import { isOSX, isWindows, isLinux } from './helpers'; @@ -82,6 +83,6 @@ export function inferFlashPath() { return findFlashOnLinux(); } - console.warn('Unable to determine OS to infer flash player'); + log.warn('Unable to determine OS to infer flash player'); return null; } diff --git a/app/src/main.ts b/app/src/main.ts index 9656c28..c8887a1 100644 --- a/app/src/main.ts +++ b/app/src/main.ts @@ -29,6 +29,10 @@ if (require('electron-squirrel-startup')) { app.exit(); } +if (process.argv.indexOf('--verbose') > -1) { + log.setLevel('DEBUG'); +} + const appArgs = JSON.parse(fs.readFileSync(APP_ARGS_FILE_PATH, 'utf8')); log.debug('appArgs', appArgs); @@ -48,9 +52,9 @@ if (process.argv.length > 1) { try { new URL(maybeUrl); appArgs.targetUrl = maybeUrl; - console.info('Loading override URL passed as argument:', maybeUrl); + log.info('Loading override URL passed as argument:', maybeUrl); } catch (err) { - console.error( + log.error( 'Not loading override URL passed as argument, because failed to parse:', maybeUrl, ); @@ -147,13 +151,14 @@ const setDockBadge = isOSX() : () => undefined; app.on('window-all-closed', () => { - log.debug('windows-all-closed'); + log.debug('app.window-all-closed'); if (!isOSX() || appArgs.fastQuit) { app.quit(); } }); app.on('activate', (event, hasVisibleWindows) => { + log.debug('app.activate', { event, hasVisibleWindows }); if (isOSX()) { // this is called when the dock is clicked if (!hasVisibleWindows) { @@ -163,7 +168,7 @@ app.on('activate', (event, hasVisibleWindows) => { }); app.on('before-quit', () => { - log.debug('before-quit'); + log.debug('app.before-quit'); // not fired when the close button on the window is clicked if (isOSX()) { // need to force a quit as a workaround here to simulate the osx app hiding behaviour @@ -176,15 +181,16 @@ app.on('before-quit', () => { }); app.on('will-quit', (event) => { - log.debug('will-quit', event); + log.debug('app.will-quit', event); }); app.on('quit', (event, exitCode) => { - log.debug('quit', event, exitCode); + log.debug('app.quit', { event, exitCode }); }); if (appArgs.crashReporter) { app.on('will-finish-launching', () => { + log.debug('app.will-finish-launching'); crashReporter.start({ companyName: appArgs.companyName || '', productName: appArgs.name, @@ -200,6 +206,7 @@ if (shouldQuit) { app.quit(); } else { app.on('second-instance', () => { + log.debug('app.second-instance'); if (mainWindow) { if (!mainWindow.isVisible()) { // try @@ -216,7 +223,7 @@ if (shouldQuit) { if (appArgs.widevine) { // @ts-ignore This event only appears on the widevine version of electron, which we'd see at runtime app.on('widevine-ready', (version: string, lastVersion: string) => { - console.log('widevine-ready', version, lastVersion); + log.debug('app.widevine-ready', { version, lastVersion }); onReady(); }); @@ -224,19 +231,20 @@ if (shouldQuit) { // @ts-ignore This event only appears on the widevine version of electron, which we'd see at runtime 'widevine-update-pending', (currentVersion: string, pendingVersion: string) => { - console.log( - `Widevine ${currentVersion} is ready to be upgraded to ${pendingVersion}`, - ); + log.debug('app.widevine-update-pending', { + currentVersion, + pendingVersion, + }); }, ); // @ts-ignore This event only appears on the widevine version of electron, which we'd see at runtime app.on('widevine-error', (error: any) => { - console.error('WIDEVINE ERROR: ', error); + log.error('app.widevine-error', error); }); } else { app.on('ready', () => { - console.log('ready'); + log.debug('ready'); onReady(); }); } @@ -318,10 +326,12 @@ function onReady(): void { } } app.on('new-window-for-tab', () => { + log.debug('app.new-window-for-tab'); mainWindow.emit('new-tab'); }); app.on('login', (event, webContents, request, authInfo, callback) => { + log.debug('app.login', { event, request }); // for http authentication event.preventDefault(); diff --git a/app/src/preload.ts b/app/src/preload.ts index 6fdaa04..4e13759 100644 --- a/app/src/preload.ts +++ b/app/src/preload.ts @@ -11,6 +11,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { ipcRenderer } from 'electron'; +import * as log from 'loglevel'; export const INJECT_DIR = path.join(__dirname, '..', 'inject'); @@ -52,11 +53,11 @@ function injectScripts() { ) .map((jsFileStat) => path.join('..', 'inject', jsFileStat.name)); for (const jsFile of jsFiles) { - console.log(`Injecting JS file: ${jsFile}`); + log.debug('Injecting JS file', jsFile); require(jsFile); } } catch (error) { - console.warn('Error encoutered injecting JS files', error); + log.error('Error encoutered injecting JS files', error); } } @@ -70,10 +71,11 @@ function notifyNotificationClick() { setNotificationCallback(notifyNotificationCreate, notifyNotificationClick); ipcRenderer.on('params', (event, message) => { + log.debug('ipcRenderer.params', { event, message }); const appArgs = JSON.parse(message); - console.info('nativefier.json', appArgs); + log.info('nativefier.json', appArgs); }); ipcRenderer.on('debug', (event, message) => { - console.info('debug:', message); + log.debug('ipcRenderer.debug', { event, message }); }); diff --git a/package.json b/package.json index e14b7c1..305e179 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "ci": "npm run lint && npm test", "clean": "rimraf lib/ app/lib/ app/dist/", "clean:full": "rimraf lib/ app/lib/ app/dist/ app/node_modules/ node_modules/", - "lint:fix": "eslint . --fix", + "lint:fix": "eslint . --ext .ts --fix", "lint:format": "prettier --write 'src/**/*.js' 'app/src/**/*.js'", "lint": "eslint . --ext .ts", "list-outdated-deps": "npm out; cd app && npm out; true", diff --git a/src/build/prepareElectronApp.ts b/src/build/prepareElectronApp.ts index 3a014bd..84fe9a3 100644 --- a/src/build/prepareElectronApp.ts +++ b/src/build/prepareElectronApp.ts @@ -115,7 +115,7 @@ async function maybeCopyScripts(srcs: string[], dest: string): Promise { } if (supportedInjectionExtensions.indexOf(path.extname(src)) < 0) { - console.log(`Skipping unsupported injection file: ${src}`); + log.warn('Skipping unsupported injection file', src); continue; } diff --git a/src/cli.ts b/src/cli.ts index c9573eb..71f32d5 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -310,7 +310,7 @@ if (require.main === module) { }; if (!options.targetUrl && !options.upgrade) { - console.error( + log.error( 'Nativefier must be called with either a targetUrl or the --upgrade option.', ); commander.help();