From 79d59dc66c41d65c7770b64ed7b366eeb5691ee0 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Mon, 13 Apr 2026 12:40:14 +0400 Subject: [PATCH] fix: cleanup chronicles writer in close function (#1413) Signed-off-by: Arnaud Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../requests/node_debug_request.nim | 17 ++++----- .../requests/node_lifecycle_request.nim | 5 ++- storage.nim | 6 ++-- storage/conf.nim | 35 ++++++++++++------- storage/storage.nim | 17 ++++++++- 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/library/storage_thread_requests/requests/node_debug_request.nim b/library/storage_thread_requests/requests/node_debug_request.nim index 8bf3106c..4a7ac4c6 100644 --- a/library/storage_thread_requests/requests/node_debug_request.nim +++ b/library/storage_thread_requests/requests/node_debug_request.nim @@ -52,14 +52,15 @@ proc getDebug( let node = storage[].node let table = RestRoutingTable.init(node.discovery.protocol.routingTable) - let json = %*{ - "id": $node.switch.peerInfo.peerId, - "addrs": node.switch.peerInfo.addrs.mapIt($it), - "spr": - if node.discovery.dhtRecord.isSome: node.discovery.dhtRecord.get.toURI else: "", - "announceAddresses": node.discovery.announceAddrs, - "table": table, - } + let json = + %*{ + "id": $node.switch.peerInfo.peerId, + "addrs": node.switch.peerInfo.addrs.mapIt($it), + "spr": + if node.discovery.dhtRecord.isSome: node.discovery.dhtRecord.get.toURI else: "", + "announceAddresses": node.discovery.announceAddrs, + "table": table, + } return ok($json) diff --git a/library/storage_thread_requests/requests/node_lifecycle_request.nim b/library/storage_thread_requests/requests/node_lifecycle_request.nim index 31da9416..e5e5e524 100644 --- a/library/storage_thread_requests/requests/node_lifecycle_request.nim +++ b/library/storage_thread_requests/requests/node_lifecycle_request.nim @@ -91,7 +91,6 @@ proc createStorage( configJson: cstring ): Future[Result[StorageServer, string]] {.async: (raises: []).} = var conf: StorageConf - try: conf = StorageConf.load( version = storageFullVersion, @@ -107,7 +106,7 @@ proc createStorage( except ConfigurationError as e: return err("Failed to create Storage: unable to load configuration: " & e.msg) - conf.setupLogging() + let logFile = conf.setupLogging() try: {.gcsafe.}: @@ -149,7 +148,7 @@ proc createStorage( let server = try: - StorageServer.new(conf, pk) + StorageServer.new(conf, pk, logFile) except Exception as exc: return err("Failed to create Storage: " & exc.msg) diff --git a/storage.nim b/storage.nim index d8d2bfa0..17ec3d2a 100644 --- a/storage.nim +++ b/storage.nim @@ -53,7 +53,8 @@ when isMainModule: sources.addConfigFile(Toml, configFile) , ) - config.setupLogging() + + let logFile = config.setupLogging() try: updateLogLevel(config.logLevel) @@ -92,9 +93,10 @@ when isMainModule: config.dataDir / config.netPrivKeyFile privateKey = setupKey(keyPath).expect("Should setup private key!") + server = try: - StorageServer.new(config, privateKey) + StorageServer.new(config, privateKey, logFile) except Exception as exc: error "Failed to start Logos Storage", msg = exc.msg quit QuitFailure diff --git a/storage/conf.nim b/storage/conf.nim index f83d0bd7..47efdde8 100644 --- a/storage/conf.nim +++ b/storage/conf.nim @@ -542,11 +542,27 @@ proc updateLogLevel*(logLevel: string) {.raises: [ValueError].} = if not setTopicState(topicName, settings.state, settings.logLevel): warn "Unrecognized logging topic", topic = topicName -proc setupLogging*(conf: StorageConf) = +proc openLogFile(conf: StorageConf): Option[IoHandle] = + if logFilePath =? conf.logFile and logFilePath.len > 0: + let logFileHandle = + openFile(logFilePath, {OpenFlags.Write, OpenFlags.Create, OpenFlags.Truncate}) + if logFileHandle.isErr: + error "failed to open log file", + path = logFilePath, errorCode = $logFileHandle.error + else: + return logFileHandle.option + return IoHandle.none + +proc setupLogging*(conf: StorageConf): Option[IoHandle] = + let ioHandle = + if conf.logFile.isSome: + conf.openLogFile() + else: + IoHandle.none + when defaultChroniclesStream.outputs.type.arity != 3: warn "Logging configuration options not enabled in the current build" else: - var logFile: ?IoHandle proc noOutput(logLevel: LogLevel, msg: LogOutputStr) = discard @@ -564,20 +580,13 @@ proc setupLogging*(conf: StorageConf) = writeAndFlush(stdout, stripAnsi(msg)) proc fileFlush(logLevel: LogLevel, msg: LogOutputStr) = - if file =? logFile: + if file =? ioHandle: if error =? file.writeFile(stripAnsi(msg).toBytes).errorOption: error "failed to write to log file", errorCode = $error defaultChroniclesStream.outputs[2].writer = noOutput - if logFilePath =? conf.logFile and logFilePath.len > 0: - let logFileHandle = - openFile(logFilePath, {OpenFlags.Write, OpenFlags.Create, OpenFlags.Truncate}) - if logFileHandle.isErr: - error "failed to open log file", - path = logFilePath, errorCode = $logFileHandle.error - else: - logFile = logFileHandle.option - defaultChroniclesStream.outputs[2].writer = fileFlush + if ioHandle.isSome: + defaultChroniclesStream.outputs[2].writer = fileFlush defaultChroniclesStream.outputs[1].writer = noOutput @@ -606,6 +615,8 @@ proc setupLogging*(conf: StorageConf) = else: defaultChroniclesStream.outputs[0].writer = writer + return ioHandle + proc setupMetrics*(config: StorageConf) = if config.metricsEnabled: let metricsAddress = config.metricsAddress diff --git a/storage/storage.nim b/storage/storage.nim index f7766fb3..97bbaf53 100644 --- a/storage/storage.nim +++ b/storage/storage.nim @@ -42,6 +42,7 @@ logScope: type StorageServer* = ref object config: StorageConf + logFile*: Option[IoHandle] restServer: RestServerRef storageNode: StorageNodeRef repoStore: RepoStore @@ -124,6 +125,16 @@ proc close*(s: StorageServer) {.async.} = error "Failed to stop the taskpool", failures = res.failure.len raiseAssert("Failure in taskpool shutdown:" & exc.msg) + when defaultChroniclesStream.outputs.type.arity >= 3: + proc noOutput(logLevel: LogLevel, msg: LogOutputStr) = + discard + + defaultChroniclesStream.outputs[2].writer = noOutput + + if s.logFile.isSome: + if error =? closeFile(s.logFile.get()).errorOption: + error "Failed to close log file", errorCode = $error + if res.failure.len > 0: error "Failed to close Storage node", failures = res.failure.len raiseAssert "Failed to close Storage node" @@ -133,7 +144,10 @@ proc shutdown*(server: StorageServer) {.async.} = await server.close() proc new*( - T: type StorageServer, config: StorageConf, privateKey: StoragePrivateKey + T: type StorageServer, + config: StorageConf, + privateKey: StoragePrivateKey, + logFile: Option[IoHandle] = IoHandle.none, ): StorageServer = ## create StorageServer including setting up datastore, repostore, etc let listenMultiAddr = getMultiAddrWithIpAndTcpPort(config.listenIp, config.listenPort) @@ -272,4 +286,5 @@ proc new*( repoStore: repoStore, maintenance: maintenance, taskPool: taskPool, + logFile: logFile, )