fix: address review comments for logging and tests

Author: Priyanshubhartistm

src/factories/logger-factory.ts

@@ -47,6 +47,26 @@ const logAtLevel = (
     return
   }
 
+  const errorFromArgs = args.find((arg) => arg instanceof Error) as Error | undefined
+
+  if (errorFromArgs) {
+    if (typeof message === 'string') {
+      instance[level]({ err: errorFromArgs }, safeFormat(message, args))
+      return
+    }
+
+    const data = [message, ...args].filter((arg) => !(arg instanceof Error))
+    const formatted = data.map(stringifyForLog).join(' ')
+
+    if (formatted) {
+      instance[level]({ err: errorFromArgs }, formatted)
+    } else {
+      instance[level]({ err: errorFromArgs })
+    }
+
+    return
+  }
+
   if (typeof message === 'string') {
     instance[level](safeFormat(message, args))
     return
@@ -87,7 +107,7 @@ const createMessageLogger = (instance: PinoLogger, scope: string): MessageLogger
 
 export const createLogger = (
   namespace: string,
-  options: { enabled?: boolean; stdout?: boolean } = { enabled: false, stdout: false },
+  options: { enabled?: boolean } = { enabled: false },
 ) => {
   const prefix = cluster.isWorker ? (process.env.WORKER_TYPE ?? 'worker') : 'primary'
   const scope = namespace ? `${prefix}:${namespace}` : prefix

src/repositories/invoice-repository.ts

@@ -23,7 +23,7 @@ export class InvoiceRepository implements IInvoiceRepository {
     try {
       await client.raw('select confirm_invoice(?, ?, ?)', [invoiceId, amountPaid.toString(), confirmedAt.toISOString()])
     } catch (error) {
-      logger.error('Unable to confirm invoice. Reason:', error.message)
+      logger.error('Unable to confirm invoice. Reason:', error)
 
       throw error
     }

src/services/payments-service.ts

@@ -29,7 +29,7 @@ export class PaymentsService implements IPaymentsService {
     try {
       return await this.invoiceRepository.findPendingInvoices(0, 10)
     } catch (error) {
-      logger.info('Unable to get pending invoices.', error)
+      logger.error('Unable to get pending invoices.', error)
 
       throw error
     }
@@ -41,7 +41,7 @@ export class PaymentsService implements IPaymentsService {
         typeof invoice === 'string' || invoice?.verifyURL ? invoice : invoice.id,
       )
     } catch (error) {
-      logger.info('Unable to get invoice from payments processor. Reason:', error)
+      logger.error('Unable to get invoice from payments processor. Reason:', error)
 
       throw error
     }

src/tor/client.ts

@@ -204,7 +204,6 @@ export const addOnion = async (port: number, host?: string): Promise<string> =>
 
     if (hiddenService?.PrivateKey) {
       logger.info('saving private key to %s', path)
-      logger('saving private key to %s', path)
 
       await writeFile(path, hiddenService.PrivateKey, 'utf8')
       return hiddenService.ServiceID!

test/unit/factories/logger-factory.spec.ts

@@ -67,6 +67,28 @@ describe('createLogger', () => {
     Sinon.assert.calledOnceWithExactly(rootInstance.error, { err: error })
   })
 
+  it('logs structured err when Error is passed as an arg to string message', () => {
+    const logger = createLogger('payments')
+    const error = new Error('boom')
+
+    logger.error('Unable to create invoice. Reason:', error)
+
+    Sinon.assert.calledOnce(rootInstance.error)
+    Sinon.assert.calledWith(rootInstance.error, { err: error })
+    expect(rootInstance.error.firstCall.args[1]).to.be.a('string').and.include('Unable to create invoice. Reason:')
+  })
+
+  it('keeps non-error args in message while logging structured err', () => {
+    const logger = createLogger('payments')
+    const error = new Error('boom')
+
+    logger.error('error handling message', { kind: 1 }, error)
+
+    Sinon.assert.calledOnce(rootInstance.error)
+    Sinon.assert.calledWith(rootInstance.error, { err: error })
+    expect(rootInstance.error.firstCall.args[1]).to.be.a('string').and.include("kind: 1")
+  })
+
   it('logs mixed non-string messages safely', () => {
     const logger = createLogger('payments')
 

test/unit/services/payments-service.spec.ts

@@ -463,7 +463,7 @@ describe('PaymentsService', () => {
       expect(eventUtils.broadcastEvent as Sinon.SinonStub).to.have.been.calledOnce
     })
 
-    it('calls logError and does not throw when the pipeline fails', async () => {
+    it('does not throw when the pipeline fails', async () => {
       ;(eventUtils.identifyEvent as Sinon.SinonStub).rejects(new Error('identify failed'))
 
       // otherwise() swallows the error — the method must resolve, not reject

test/unit/utils/settings.spec.ts

@@ -198,7 +198,11 @@ describe('SettingsStatic', () => {
 
       expect(SettingsStatic.createSettings()).to.be.an('object')
 
-      expect(existsSyncStub).to.have.been.calledWithExactly('/some/path/settings.json')
+      const settingsPathExistsChecks = existsSyncStub.getCalls().filter((call) => {
+        return call.args.length === 1 && call.args[0] === '/some/path/settings.json'
+      })
+
+      expect(settingsPathExistsChecks).to.have.lengthOf(1)
       expect(getSettingsFileBasePathStub).to.have.been.calledOnce
       expect(saveSettingsStub).to.have.been.calledOnceWithExactly('/some/path/settings.json', Sinon.match.object)
       expect(loadSettingsStub).to.have.been.called