Skip to content
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.

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

Log refactoring #193

Merged
merged 11 commits into from
May 6, 2021
7 changes: 6 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@ module.exports = {
'arrow-body-style': 'off',
'object-curly-newline': 'warn',
'semi': productionError,
'no-return-assign': 'off'
'no-return-assign': 'off',
'no-restricted-imports': ['error', {
name: 'electron-log',
importNames: ['default'],
message: 'You should use logging/logger instead'
}]
},

overrides: [
Expand Down
2 changes: 1 addition & 1 deletion src/backend/consts.ts → src/app-globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { app, remote } from 'electron';
import { mkdirSync } from 'fs';
import path from 'path';

const App = app || remote.app;
export const App = app || remote.app;

if (process.env.NODE_ENV !== 'production') {
const localUserData = path.resolve('userData');
Expand Down
11 changes: 0 additions & 11 deletions src/backend/eventEmitters/compositeEventPublisher.ts

This file was deleted.

11 changes: 0 additions & 11 deletions src/backend/eventEmitters/consoleEmitter.ts

This file was deleted.

13 changes: 13 additions & 0 deletions src/backend/eventEmitters/loggerEmitter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { BudgetTrackingEventEmitter, EventPublisher } from '@/backend/eventEmitters/EventEmitter';

interface Logger {
info: (...params: any[]) => void
}

export function buildLoggerEmitter(logger: Logger): EventPublisher {
const loggerEmitter = new BudgetTrackingEventEmitter();
loggerEmitter.onAny((eventName, eventData) => {
logger.info(`${eventName}:`, eventData);
});
return loggerEmitter;
}
Comment on lines +1 to +13
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a very smart method, but it is here for a reason.

Here, in the eventEmitters folder, in the context of the eventEmitters, we know the event types, so we can parse them to string (with special care for each event), and direct them to the right level.

So, continuously, we can use this method to

  1. Print specific message depends on the Event.
  2. Print to the console or to the file, by level (because the logger argument supports both console.[level] and logger.[level]

2 changes: 1 addition & 1 deletion src/backend/import/importTransactions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { userDataPath } from '@/app-globals';
import { EnrichedTransaction } from '@/backend/commonTypes';
import * as configManager from '@/backend/configManager/configManager';
import { userDataPath } from '@/backend/consts';
import * as bankScraper from '@/backend/import/bankScraper';
import { ScaperScrapingResult, Transaction } from '@/backend/import/bankScraper';
import * as categoryCalculation from '@/backend/import/categoryCalculationScript';
Expand Down
13 changes: 1 addition & 12 deletions src/backend/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import * as configManager from './configManager/configManager';
import {
BudgetTrackingEvent, BudgetTrackingEventEmitter, EventNames, EventPublisher
} from './eventEmitters/EventEmitter';
import { buildCompositeEventPublisher } from './eventEmitters/compositeEventPublisher';
import { buildConsoleEmitter } from './eventEmitters/consoleEmitter';
import outputVendors from './export/outputVendors';
import * as bankScraper from './import/bankScraper';

Expand All @@ -18,7 +16,7 @@ export { BudgetTrackingEvent, BudgetTrackingEventEmitter };
export const { inputVendors } = bankScraper;

export async function scrapeAndUpdateOutputVendors(config: configManager.Config, optionalEventPublisher?: EventPublisher) {
const eventPublisher = createEventPublisher(optionalEventPublisher);
const eventPublisher = optionalEventPublisher || new BudgetTrackingEventEmitter();

const startDate = moment()
.subtract(config.scraping.numDaysBack, 'days')
Expand All @@ -37,12 +35,3 @@ export async function scrapeAndUpdateOutputVendors(config: configManager.Config,
throw e;
}
}

function createEventPublisher<Name>(optionalEventPublisher: EventPublisher | undefined): EventPublisher {
const eventPublishers = [buildConsoleEmitter()];
if (optionalEventPublisher) {
eventPublishers.push(optionalEventPublisher);
}
const compositeEventPublisher = buildCompositeEventPublisher(eventPublishers);
return compositeEventPublisher;
}
5 changes: 1 addition & 4 deletions src/background.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { app, BrowserWindow } from 'electron';
// eslint-disable-next-line import/no-extraneous-dependencies
import { createProtocol, installVueDevtools } from 'vue-cli-plugin-electron-builder/lib';
import CreateLogger from './logging/logger';
import logger from './logging/logger';
import Sentry from './logging/sentry';
// import './store';

Expand All @@ -13,9 +13,6 @@ const isDevelopment = process.env.NODE_ENV !== 'production';
// be closed automatically when the JavaScript object is garbage collected.
let mainWindow: BrowserWindow | null;

global.logger = CreateLogger(app);
const { logger } = global;

function createWindow() {
// Create the browser window.
mainWindow = new BrowserWindow({
Expand Down
File renamed without changes.
39 changes: 17 additions & 22 deletions src/logging/logger.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,24 @@
import { App } from 'electron';
import logger from 'electron-log';
import { App } from '@/app-globals';
import logger, { LogFunctions, LogLevel } from 'electron-log'; // eslint-disable-line no-restricted-imports
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you will try to import logger from 'electron-log', you will get an ESLint error, because we want you to use our custom-defined logger.
But here we have to import the logger from electron-log to configure it.

import fs from 'fs';
import { EOL } from 'os';

declare module 'electron-log' {
interface ElectronLog {
getLastLines: (n: number) => string
}
}
export type Logger = LogFunctions
export type Levels = LogLevel

export default function CreateLogger(app: App) {
logger.info(`Welcome to ${app.getName()} log`);
logger.info(`Version: ${app.getVersion()}`);
logger.info(`Welcome to ${App.getName()} log`);
logger.info(`Version: ${App.getVersion()}`);

const onError = (error: Error) => {
logger.error(error.message ? error.message : error);
if (error.stack) logger.debug(error.stack);
};
logger.catchErrors({ onError });
const onError = (error: Error) => {
logger.error(error.message || error);
if (error.stack) logger.debug(error.stack);
};
logger.catchErrors({ onError });
Comment on lines +13 to +17
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, this catching only the UnhandledRejection.

At least for the Vue part, we need to open a new issue to catch exceptions during the render process (I even can't say what exactly happened during such case. For in React, for example, the component tree will crash- if you're not using error boundary)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


logger.getLastLines = (n) => {
const lines = fs.readFileSync(logger.transports.file.getFile().path).toString().split(EOL);
const lastLines = lines.slice(lines.length - n);
return lastLines.join(EOL);
};
export const getLastLines = (n: number) => {
const lines = fs.readFileSync(logger.transports.file.getFile().path).toString().split(EOL);
const lastLines = lines.slice(lines.length - n);
return lastLines.join(EOL);
};

return logger;
}
export default logger as Logger;
9 changes: 2 additions & 7 deletions src/main.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import VueCompositionAPI, { h, ref } from '@vue/composition-api';
import electron from 'electron';
import { ElectronLog } from 'electron-log';
import Vue from 'vue';
import logger from './logging/logger';
import Sentry from './logging/sentry';
import App from './ui/components/App.vue';
import SplashScreen from './ui/components/SplashScreen.vue';
import LoggerPlugin from './ui/plugins/logger';
import vuetify from './ui/plugins/vuetify';
import router from './ui/router';
import store from './ui/store';
Expand All @@ -17,10 +15,6 @@ process.on('unhandledRejection', (error) => {
console.error(error);
});

const logger: ElectronLog = electron.remote.getGlobal('logger');
logger.info('The renderer process got the logger');
Vue.use(LoggerPlugin, { logger });

Vue.use(VueCompositionAPI);

Vue.config.productionTip = process.env.NODE_ENV !== 'production';
Expand All @@ -33,6 +27,7 @@ new Vue({
name: APP_NAME,

setup(_, { root }) {
logger.info('Vue started');
const loaded = ref(false);
root.$store.restored.then(() => {
loaded.value = true;
Expand Down
2 changes: 1 addition & 1 deletion src/manual/setupHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-console */
import { getConfig } from '@/backend/configManager/configManager';
import { configFilePath } from '@/backend/consts';
import { configFilePath } from '@/app-globals';
import { getYnabAccountDetails } from '@/backend/export/outputVendors/ynab/ynab';
import { getFinancialAccountNumbers } from '@/backend/import/importTransactions';

Expand Down
2 changes: 1 addition & 1 deletion src/ui/components/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
import { shell } from 'electron';
import Importers from '@/ui/components/app/Importers';
import Exporters from '@/ui/components/app/Exporters';
import { trackPage } from '@/analytics';
import { trackPage } from '@/logging/analytics';
import ReportProblemDialog from './app/ReportProblemDialog';
import MainContent from './app/MainContent';
import { repository } from '../../../package.json';
Expand Down
7 changes: 4 additions & 3 deletions src/ui/components/app/MainContent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ import { scrapeAndUpdateOutputVendors, BudgetTrackingEventEmitter } from '@/back
import { AccountsState, handleEvent } from '@/ui/components/app/accountsState';
import store from '@/ui/store';
import ConfigEditor from '@/ui/components/app/ConfigEditor.vue';
import { initAnalyticsEventHandling } from '@/analytics';
import { Levels } from '../shared/log/types';
import { initAnalyticsEventHandling } from '@/logging/analytics';
import logger, { Levels } from '@/logging/logger';

const statusToColor = {
NOT_STARTED: null,
Expand Down Expand Up @@ -117,7 +117,8 @@ export default defineComponent({
function initEventHandlers(eventEmitter: BudgetTrackingEventEmitter, accountsState: Ref<UnwrapRef<AccountsState>>) {
eventEmitter.onAny((eventName, eventData) => {
const message = eventData?.message || eventName;
const logLevel = eventData?.error ? Levels.Error : Levels.Info;
const logLevel: Levels = eventData?.error ? 'error' : 'info';
logger[logLevel](message);
return handleEvent({ ...eventData, message, level: logLevel }, accountsState.value);
});
initAnalyticsEventHandling(eventEmitter);
Expand Down
3 changes: 2 additions & 1 deletion src/ui/components/app/ReportProblemDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import { shell } from 'electron';
import Sentry from '@/logging/sentry';
import LogSheet from '@/ui/components/shared/LogSheet';
import os from 'os';
import { getLastLines } from '@/logging/logger';
import { repository } from '../../../../package.json';

const createGithubIssueLink = (title, details, log) => {
Expand Down Expand Up @@ -193,7 +194,7 @@ export default {
}
},
resetForm() {
this.raw = this.$logger.getLastLines(10);
this.raw = getLastLines(10);
this.formData = { ...defaultFormData };
this.validateEmail = false;
},
Expand Down
2 changes: 1 addition & 1 deletion src/ui/components/app/accountsState.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// eslint-disable-next-line max-classes-per-file
import { BudgetTrackingEvent } from '@/backend';
import { Levels } from '@/ui/components/shared/log/types';
import { Levels } from '@/logging/logger';
import { UnwrapRef } from '@vue/composition-api';
import { AccountStatus, AccountType } from '@/backend/eventEmitters/EventEmitter';

Expand Down
Empty file.
10 changes: 5 additions & 5 deletions src/ui/components/shared/log/LogViewer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@
import { defineComponent, PropType } from '@vue/composition-api';
import { AccountsState } from '@/ui/components/app/accountsState';
import AccountCard from '@/ui/components/shared/log/AccountCard.vue';
import { Levels } from './types';
import { Levels } from '@/logging/logger';

const levelToClass = {
[Levels.Error]: 'error-line',
[Levels.Warn]: 'warn-line',
[Levels.Info]: 'info-line'
error: 'error-line',
warn: 'warn-line',
info: 'info-line'
};

export default defineComponent({
Expand All @@ -44,7 +44,7 @@ export default defineComponent({
},
},
setup() {
const getClass = (level: Levels) => levelToClass[level];
const getClass = (level: Levels):string => levelToClass[level];
return { getClass };
}
});
Expand Down
8 changes: 0 additions & 8 deletions src/ui/components/shared/log/types.ts

This file was deleted.

5 changes: 0 additions & 5 deletions src/ui/plugins/logger.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/ui/store/plugins/persisted-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { configManager } from '@/backend';
import { configFilePath } from '@/backend/consts';
import { configFilePath } from '@/app-globals';
import VuexPersistence from 'vuex-persist';

export default (configModuleName: string) => new VuexPersistence<any>({
Expand Down