fix: cleanup chronicles writer in close function (#1413)

Signed-off-by: Arnaud <arno.deville@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

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)
 

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)
 

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

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

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,
   )