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

Renderer file logging through IPC #7499

Merged
merged 14 commits into from Apr 4, 2023
Merged

Renderer file logging through IPC #7499

merged 14 commits into from Apr 4, 2023

Conversation

samitiilikainen
Copy link
Contributor

Implementation similar to #6795, produces the same log files.

To avoid using file system directly in renderer, now using IPC transport to send log messages from renderer to main. Main then writes messages to files using the file transport of Winston, ensuring there is at most one file handle open for each log file.

Unfortunately also the same UI reload/quit issues apply here as in #6795 (reverted in #7245).

  1. UI freezes if there is any pagehide listener in cluster frame - at least in certain situations with multiple clusters open.
  2. This forces to use beforeunload as that doesn't freeze the UI, but also doesn't often get called in all cluster frames when main frame is unmounted.
  3. Cluster log files are left open when beforeunload is not called

To fix issues leaving files open:

  1. This implementation creates only one file handle per cluster id in main, so when UI is reloaded, the same unclosed files will be reused.
  2. When only the UI is quit (leaving the main process running), there is now stopIpcLoggingInjectable with beforeQuitOfFrontEndInjectionToken to ensure all renderer log files are closed.

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Pagehide was needed in cluster frame to better handle main frame close/reload situation. But even empty pagehide listener in cluster frame seems to freeze the UI at least on some situations (multiple clusters open).

Beforeunload is not always executed in cluster frame when main frame is reloaded/closed, leaving log files open. To fix that, `stopIpcLoggingInjectable` is introduced to close all log files.

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>
@samitiilikainen samitiilikainen requested a review from a team as a code owner April 4, 2023 07:38
@samitiilikainen samitiilikainen requested review from aleksfront and removed request for a team April 4, 2023 07:38
@Nokel81 Nokel81 added the enhancement New feature or request label Apr 4, 2023
@Nokel81 Nokel81 added this to the 6.5.0 milestone Apr 4, 2023
@Nokel81 Nokel81 merged commit 69b1323 into master Apr 4, 2023
15 of 16 checks passed
@Nokel81 Nokel81 deleted the feature/ipc-logging branch April 4, 2023 12:11
@Nokel81 Nokel81 mentioned this pull request Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants