From da946cf79381e542e9789739205b86474c50d9fc Mon Sep 17 00:00:00 2001 From: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> Date: Fri, 23 Dec 2022 13:26:26 +0200 Subject: [PATCH] Improve file handle closing in different situations This should cover reloading main and cluster frames and closing cluster frame throught disconnecting cluster. No file handles should be left open now. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> --- packages/core/src/common/logger.injectable.ts | 11 ++-------- .../src/common/winston-logger.injectable.ts | 20 +++++++++++++++++++ packages/core/src/renderer/bootstrap.tsx | 11 ++-------- .../init-cluster-frame.injectable.ts | 2 ++ .../init-cluster-frame/init-cluster-frame.ts | 12 ++++++++--- .../root-frame/init-root-frame.injectable.ts | 4 +++- .../close-renderer-log-file.injectable.ts | 3 +++ 7 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 packages/core/src/common/winston-logger.injectable.ts diff --git a/packages/core/src/common/logger.injectable.ts b/packages/core/src/common/logger.injectable.ts index 8e9dd2a6a7..82ff682c46 100644 --- a/packages/core/src/common/logger.injectable.ts +++ b/packages/core/src/common/logger.injectable.ts @@ -3,20 +3,13 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; -import { createLogger, format } from "winston"; import type { Logger } from "./logger"; -import { loggerTransportInjectionToken } from "./logger/transports"; +import winstonLoggerInjectable from "./winston-logger.injectable"; const loggerInjectable = getInjectable({ id: "logger", instantiate: (di): Logger => { - const baseLogger = createLogger({ - format: format.combine( - format.splat(), - format.simple(), - ), - transports: di.injectMany(loggerTransportInjectionToken), - }); + const baseLogger = di.inject(winstonLoggerInjectable); return { debug: (message, ...data) => baseLogger.debug(message, ...data), diff --git a/packages/core/src/common/winston-logger.injectable.ts b/packages/core/src/common/winston-logger.injectable.ts new file mode 100644 index 0000000000..ec3854d8b9 --- /dev/null +++ b/packages/core/src/common/winston-logger.injectable.ts @@ -0,0 +1,20 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { createLogger, format } from "winston"; +import { loggerTransportInjectionToken } from "./logger/transports"; + +const winstonLoggerInjectable = getInjectable({ + id: "winston-logger", + instantiate: (di) => createLogger({ + format: format.combine( + format.splat(), + format.simple(), + ), + transports: di.injectMany(loggerTransportInjectionToken), + }), +}); + +export default winstonLoggerInjectable; diff --git a/packages/core/src/renderer/bootstrap.tsx b/packages/core/src/renderer/bootstrap.tsx index 3fc23120a6..f74c74986f 100644 --- a/packages/core/src/renderer/bootstrap.tsx +++ b/packages/core/src/renderer/bootstrap.tsx @@ -19,7 +19,6 @@ import { Router } from "react-router"; import historyInjectable from "./navigation/history.injectable"; import assert from "assert"; import startFrameInjectable from "./start-frame/start-frame.injectable"; -import closeRendererLogFileInjectable from "./logger/close-renderer-log-file.injectable"; export async function bootstrap(di: DiContainer) { const startFrame = di.inject(startFrameInjectable); @@ -55,15 +54,9 @@ export async function bootstrap(di: DiContainer) { } try { - const unmount = () => { - const closeLogFile = di.inject(closeRendererLogFileInjectable); - - closeLogFile(); - + await initializeApp(() => { unmountComponentAtNode(rootElem); - }; - - await initializeApp(unmount); + }); } catch (error) { console.error(`[BOOTSTRAP]: view initialization error: ${error}`, { origin: location.href, diff --git a/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.injectable.ts b/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.injectable.ts index c640264ee3..a9a923f860 100644 --- a/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.injectable.ts +++ b/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.injectable.ts @@ -12,6 +12,7 @@ import emitAppEventInjectable from "../../../../common/app-event-bus/emit-event. import loadExtensionsInjectable from "../../load-extensions.injectable"; import loggerInjectable from "../../../../common/logger.injectable"; import showErrorNotificationInjectable from "../../../components/notifications/show-error-notification.injectable"; +import closeRendererLogFileInjectable from "../../../logger/close-renderer-log-file.injectable"; const initClusterFrameInjectable = getInjectable({ id: "init-cluster-frame", @@ -29,6 +30,7 @@ const initClusterFrameInjectable = getInjectable({ emitAppEvent: di.inject(emitAppEventInjectable), logger: di.inject(loggerInjectable), showErrorNotification: di.inject(showErrorNotificationInjectable), + closeFileLogging: di.inject(closeRendererLogFileInjectable), }); }, }); diff --git a/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts b/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts index f81972eae3..3be2be9754 100644 --- a/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts +++ b/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts @@ -2,6 +2,7 @@ * Copyright (c) OpenLens Authors. All rights reserved. * Licensed under MIT License. See LICENSE in root directory for more information. */ +import { once } from "lodash"; import type { Cluster } from "../../../../common/cluster/cluster"; import type { CatalogEntityRegistry } from "../../../api/catalog/entity/registry"; import type { ShowNotification } from "../../../components/notifications"; @@ -18,6 +19,7 @@ interface Dependencies { emitAppEvent: EmitAppEvent; logger: Logger; showErrorNotification: ShowNotification; + closeFileLogging: () => void; } const logPrefix = "[CLUSTER-FRAME]:"; @@ -30,6 +32,7 @@ export const initClusterFrame = ({ emitAppEvent, logger, showErrorNotification, + closeFileLogging, }: Dependencies) => async (unmountRoot: () => void) => { // TODO: Make catalogEntityRegistry already initialized when passed as dependency @@ -73,11 +76,14 @@ export const initClusterFrame = ({ }); }); - window.onpagehide = () => { + const onCloseFrame = once(() => { logger.info( `${logPrefix} Unload dashboard, clusterId=${(hostedCluster.id)}, frameId=${frameRoutingId}`, ); - + closeFileLogging(); unmountRoot(); - }; + }); + + window.addEventListener("beforeunload", onCloseFrame, { capture: true }); + window.addEventListener("pagehide", onCloseFrame, { capture: true }); }; diff --git a/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts b/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts index 1732ef6249..84d62e0fdc 100644 --- a/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts +++ b/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts @@ -13,6 +13,7 @@ import loggerInjectable from "../../../common/logger.injectable"; import { delay } from "../../../common/utils"; import { broadcastMessage } from "../../../common/ipc"; import { bundledExtensionsLoaded } from "../../../common/ipc/extension-handling"; +import closeRendererLogFileInjectable from "../../logger/close-renderer-log-file.injectable"; const initRootFrameInjectable = getInjectable({ id: "init-root-frame", @@ -24,6 +25,7 @@ const initRootFrameInjectable = getInjectable({ const lensProtocolRouterRenderer = di.inject(lensProtocolRouterRendererInjectable); const catalogEntityRegistry = di.inject(catalogEntityRegistryInjectable); const logger = di.inject(loggerInjectable); + const closeRendererLogFile = di.inject(closeRendererLogFileInjectable); return async (unmountRoot: () => void) => { catalogEntityRegistry.init(); @@ -59,7 +61,7 @@ const initRootFrameInjectable = getInjectable({ window.addEventListener("pagehide", () => { logger.info("[ROOT-FRAME]: Unload app"); - + closeRendererLogFile(); unmountRoot(); }); }; diff --git a/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts b/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts index b47e2572a8..8480589c39 100644 --- a/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts +++ b/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts @@ -3,15 +3,18 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; +import winstonLoggerInjectable from "../../common/winston-logger.injectable"; import rendererFileLoggerTransportInjectable from "./file-transport.injectable"; const closeRendererLogFileInjectable = getInjectable({ id: "close-renderer-log-file", instantiate: (di) => { + const winstonLogger = di.inject(winstonLoggerInjectable); const fileLoggingTransport = di.inject(rendererFileLoggerTransportInjectable); return () => { fileLoggingTransport.close?.(); + winstonLogger.remove(fileLoggingTransport); }; }, });