Log refactoring #193

baruchiro · 2021-02-25T19:08:38Z

Close #141

A user should be able to send us the logs in case he faces an error.

Make the logger write to a file

Verify there is a global exception handler that logs exceptions

Add to the readme a description of how to send logs (either manual or automatic)

Make sure we printing things to the log alongside the UI (scraping events, for example)

Make sure we are re-assigning the console.log function, or we linting to not use it and use our custom logger instead.

Use different logger instance between main/renderer Remove EventEmitter builders and keep only one Do not initilize default EventEmitter

baruchiro · 2021-02-25T19:22:57Z

src/backend/eventEmitters/compositeEventPublisher.ts

-import { BudgetTrackingEventEmitter, EventPublisher } from './EventEmitter';
-
-export function buildCompositeEventPublisher(eventPublishers: EventPublisher[]): EventPublisher {
-  const compositeEmitter = new BudgetTrackingEventEmitter();
-
-  compositeEmitter.onAny(((eventName, eventData) => {
-    // @ts-ignore
-    eventPublishers.map((eventPublisher) => eventPublisher.emit(eventName, eventData));
-  }));
-  return compositeEmitter;
-}


This is not a special logic, that should be part of the "eventEmitters" tools. Anyone knows how to write a function like this.

You may remember my conflict with the "EventEmitter", so I removed some codes here, to remove complexity, since all we want from the "backend user" is to give us an object that only implement an interface with emit.

baruchiro · 2021-02-25T19:24:35Z

src/backend/eventEmitters/consoleEmitter.ts

-/* eslint-disable class-methods-use-this */
-import { BudgetTrackingEventEmitter, EventPublisher } from '@/backend/eventEmitters/EventEmitter';
-
-export function buildConsoleEmitter(): EventPublisher {
-  const consoleEmitter = new BudgetTrackingEventEmitter();
-  consoleEmitter.onAny((eventName, eventData) => {
-    // eslint-disable-next-line no-console
-    console.log(`${eventName}:`, eventData);
-  });
-  return consoleEmitter;
-}


As backend (think about the option to implement CLI in the future, for example), you not sure you will print to the console, you leave it to the "backend user" decision.

But see the new loggerEmitter, it is very similar.

baruchiro · 2021-02-25T19:30:41Z

src/backend/eventEmitters/loggerEmitter.ts

+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;
+}


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

Print specific message depends on the Event.

Print to the console or to the file, by level (because the logger argument supports both console.[level] and logger.[level]

baruchiro · 2021-02-25T19:49:18Z

src/logging/logger.ts

-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


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.

baruchiro · 2021-03-05T12:58:25Z

src/logging/logger.ts

+const onError = (error: Error) => {
+  logger.error(error.message || error);
+  if (error.stack) logger.debug(error.stack);
+};
+logger.catchErrors({ onError });


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)

baruchiro · 2021-03-19T13:21:50Z

README.md

+### Report a problem
+
+We are still in beta, and you may find errors.
+Please use the **REPORT A PROBLEM** button in the app to report to us.
+
+Use this button to find the **logs folder** as well.
+


Please review

baruchiro · 2021-03-19T13:22:34Z

src/ui/components/app/ReportProblemDialog.vue

+          <br>
+          <small>You can find the logs here: {{ logsDir }}</small>


baruchiro · 2021-03-19T13:25:53Z

@brafdlog Ready for review.

I skipping the two checkboxes. This PR is already big.

Of course, it is not so complicated, and I left comments about the changes, but when you're coming without context, it is difficult to understand.
But really, I just organized the code, refactor, and changed some functions and on them, I left comments.

…o/issue141

baruchiro added 3 commits February 8, 2021 22:09

Logging changes >

79606f4

Use different logger instance between main/renderer Remove EventEmitter builders and keep only one Do not initilize default EventEmitter

Use logger directly from the module

17e3b38

Merge remote-tracking branch 'origin/master' into baruchiro/issue141

4334f0a

baruchiro marked this pull request as draft February 25, 2021 19:09

baruchiro commented Feb 25, 2021

View reviewed changes

baruchiro added 2 commits March 1, 2021 21:50

ESLint message

904866e

Merge remote-tracking branch 'origin/master' into baruchiro/issue141

b46cbd9

baruchiro commented Mar 5, 2021

View reviewed changes

baruchiro mentioned this pull request Mar 5, 2021

Error boundary for the Vue components #198

Closed

baruchiro added 2 commits March 19, 2021 15:01

Merge branch 'master' into baruchiro/issue141

72b0b86

Add to the readme how to send logs

281fa20

baruchiro commented Mar 19, 2021

View reviewed changes

baruchiro marked this pull request as ready for review March 19, 2021 13:24

baruchiro requested a review from brafdlog March 19, 2021 14:10

baruchiro added this to the M1 - Make app functional for an end user milestone Mar 21, 2021

baruchiro added 4 commits April 5, 2021 09:28

Merge commit '6cfe8309769e3de1b510420d4cc199507601f97d' into baruchir…

9c7177a

…o/issue141

Merge commit '9feb7c57b4a259b530b2bfe468147e422f0f19a7' into baruchir…

6278662

…o/issue141

Merge commit '8928f2d5867a9af49db467071791da28bed1c103' into baruchir…

8ad8f28

…o/issue141

Merge branch 'master' into baruchiro/issue141

82b3f36

baruchiro enabled auto-merge April 29, 2021 19:28

baruchiro disabled auto-merge May 6, 2021 15:20

baruchiro merged commit 47e108a into master May 6, 2021

baruchiro deleted the baruchiro/issue141 branch May 6, 2021 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log refactoring #193

Log refactoring #193

baruchiro commented Feb 25, 2021 •

edited

Loading

baruchiro Feb 25, 2021

baruchiro Feb 25, 2021

baruchiro Feb 25, 2021

baruchiro Feb 25, 2021

baruchiro Feb 25, 2021

baruchiro Mar 5, 2021

baruchiro Mar 5, 2021

baruchiro Mar 19, 2021

baruchiro Mar 19, 2021

baruchiro commented Mar 19, 2021 •

edited

Loading

		<br>
		<small>You can find the logs here: {{ logsDir }}</small>

Log refactoring #193

Log refactoring #193

Conversation

baruchiro commented Feb 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baruchiro commented Mar 19, 2021 • edited Loading

baruchiro commented Feb 25, 2021 •

edited

Loading

baruchiro commented Mar 19, 2021 •

edited

Loading