Skip to content

chore: convert src/logging.{js,ts} #1907

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 15 additions & 24 deletions packages/streams/logging.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
const { FileTimestampStream } = require('file-timestamp-stream')
const path = require('path')
let debug = require('debug')('signalk:streams:logging')
const fs = require('fs')
const { readdir, unlink } = require('fs/promises')
const { isUndefined } = require('lodash')

const filenamePattern = /skserver-raw_\d\d\d\d-\d\d-\d\dT\d\d\.log/
Expand Down Expand Up @@ -52,28 +52,27 @@ class FileTimestampStreamWithDelete extends FileTimestampStream {

deleteOldFiles() {
debug(`Checking for old log files`)
listLogFiles(this.app, (err, files) => {
if (err) {
listLogFiles(this.app)
.catch((err) => {
console.error(err)
} else {
})
.then(async (files) => {
if (files.length > this.filesToKeep) {
const sortedFiles = files.sort()
const numToDelete = files.length - this.filesToKeep
debug(`Will delete ${numToDelete} files`)
for (let i = 0; i < numToDelete; i++) {
const fileName = path.join(this.fullLogDir, sortedFiles[i])
debug(`Deleting ${fileName}`)
fs.unlink(fileName, (err) => {
if (err) {
console.error(err)
} else {
debug(`${fileName} was deleted`)
}
})
try {
await unlink(fileName)
debug(`${fileName} was deleted`)
} catch (err) {
console.error(err)
}
}
}
}
})
})
}
}

Expand Down Expand Up @@ -135,15 +134,7 @@ function getFullLogDir(app, logdir) {
: path.join(app.config.configPath, logdir)
}

function listLogFiles(app, cb) {
fs.readdir(getFullLogDir(app), (err, files) => {
if (!err) {
cb(
undefined,
files.filter((filename) => filename.match(filenamePattern))
)
} else {
cb(err)
}
})
async function listLogFiles(app) {
const files = await readdir(getFullLogDir(app))
return files.filter((filename) => filename.match(filenamePattern))
}
9 changes: 7 additions & 2 deletions src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@
import { Delta, FeatureInfo, ServerAPI, SKVersion } from '@signalk/server-api'
import { FullSignalK } from '@signalk/signalk-schema'
import { EventEmitter } from 'node:events'

import { Config } from './config/config'
import DeltaCache from './deltacache'
import { WithSecurityStrategy } from './security'
import { IRouter } from 'express'

export interface ServerApp extends ServerAPI {
export interface ServerApp
extends ServerAPI,
WithSecurityStrategy,
IRouter,
WithConfig {
started: boolean
interfaces: { [key: string]: any }
intervals: NodeJS.Timeout[]
Expand Down
3 changes: 2 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import { pipedProviders } from './pipedproviders'
import { EventsActorId, WithWrappedEmitter, wrapEmitter } from './events'
import { Zones } from './zones'
const debug = createDebug('signalk-server')
import logging from './logging'

import { StreamBundle } from './streambundle'

Expand Down Expand Up @@ -95,7 +96,7 @@ class Server {
_.merge(app, opts)

load(app)
app.logging = require('./logging')(app)
app.logging = logging(app)
app.version = '0.0.1'

setupCors(app, getSecurityConfig(app))
Expand Down
48 changes: 32 additions & 16 deletions src/interfaces/logfiles.js → src/interfaces/logfiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,33 @@
* limitations under the License.
*/

const moment = require('moment')
const path = require('path')
const { getFullLogDir, listLogFiles } = require('@signalk/streams/logging')
import moment from 'moment'
import path from 'path'
import { getFullLogDir, listLogFiles } from '@signalk/streams/logging'
import { SERVERROUTESPREFIX } from '../constants'
import { ServerApp } from '../app'

module.exports = function (app) {
module.exports = function (app: ServerApp) {
return {
start: function () {
start() {
mountApi(app)
},
stop: () => undefined

stop() {}
}
}

function mountApi(app) {
function mountApi(app: ServerApp) {
app.securityStrategy.addAdminMiddleware(`${SERVERROUTESPREFIX}/logfiles/`)
app.get(`${SERVERROUTESPREFIX}/logfiles/`, function (req, res) {
listLogFiles(app, (err, files) => {
if (err) {
console.error(err)
res.status(500)
res.json('Error reading logfiles list')
return
}
app.get(`${SERVERROUTESPREFIX}/logfiles/`, async function (_, res) {
try {
const files = await listLogFiles(app)
res.json(files)
})
} catch (err) {
console.error(err)
res.status(500)
res.json('Error reading logfiles list')
}
})
app.get(`${SERVERROUTESPREFIX}/logfiles/:filename`, function (req, res) {
const sanitizedFilename = req.params.filename.replaceAll(/\.\.(\\|\/)/g, '')
Expand All @@ -63,3 +64,18 @@ function mountApi(app) {
})
})
}

// Add `res.zip` method to the Response interface
declare module 'express-serve-static-core' {
type ZipOptions = {
filename: string
files: {
path: string
name: string
}[]
}

interface Response {
zip(opts: ZipOptions): void
}
}
69 changes: 42 additions & 27 deletions src/logging.js → src/logging.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,35 @@
const debugCore = require('debug')
const moment = require('moment')
const path = require('path')
const fs = require('fs')

module.exports = function (app) {
const log = []
let debugEnabled = ''
import debugCore from 'debug'
import moment from 'moment'
import path from 'path'
import fs from 'fs'
import type { SignalKServer } from './types'
import type { WithConfig } from './app'
import type { WrappedEmitter } from './events'

export interface Logging {
getLog: () => LogMessage[]
enableDebug: (enabled: string) => boolean
getDebugSettings: () => { debugEnabled: string; rememberDebug: boolean }
rememberDebug: (enabled: boolean) => void
addDebug: (name: string) => void
removeDebug: (name: string) => void
}

type LogMessage = {
ts: string
row: string
isError?: boolean
}

export default function (
app: SignalKServer & WithConfig & WrappedEmitter
): Logging {
const log: LogMessage[] = []
let debugEnabled = process.env.DEBUG ?? ''
let rememberDebug = false
const size = 100
let debugPath

if (process.env.DEBUG) {
debugEnabled = process.env.DEBUG
}
const debugPath = path.join(app.config.configPath, 'debug')

debugPath = path.join(app.config.configPath, 'debug')
if (fs.existsSync(debugPath)) {
const enabled = fs.readFileSync(debugPath, 'utf8')
if (enabled.length > 0) {
Expand All @@ -24,8 +39,8 @@ module.exports = function (app) {
}
}

function storeOutput(output, isError) {
const data = {
function storeOutput(output: string, isError: boolean) {
const data: LogMessage = {
ts: moment().format('MMM DD HH:mm:ss'),
row: output
}
Expand All @@ -47,22 +62,22 @@ module.exports = function (app) {
const outWrite = process.stdout.write
const errWrite = process.stderr.write

process.stdout.write = function (string) {
outWrite.apply(process.stdout, arguments)
storeOutput(string, false)
process.stdout.write = function (...args) {
storeOutput(args[0].toString(), false)
return outWrite.apply(process.stdout, args as Parameters<typeof outWrite>)
}

process.stderr.write = function (string) {
errWrite.apply(process.stderr, arguments)
storeOutput(string, true)
process.stderr.write = function (...args) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't you doing function (...args: Parameters<typeof errWrite>) here? Similarly for outWrite.

Copy link
Contributor Author

@bkeepers bkeepers Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write is an overloaded function in typescript, and Parameters<typeof errWrite> only returns the first function signature, which results in a TypeScript error. See microsoft/TypeScript#32164 for more on this issue.

The ideal solution here is probably to do something like process.stderr = new Transform() { … } and piping that to the old stderr instead of just overwriting the write method, but I didn't want to make too invasive of changes here.

storeOutput(args[0].toString(), true)
return errWrite.apply(process.stderr, args as Parameters<typeof errWrite>)
}

// send debug to stdout so it does not look like an error
debugCore.log = console.info.bind(console)

function enableDebug(enabled) {
function enableDebug(enabled: string) {
if (enabled.length > 0) {
let all = enabled.split(',')
const all = enabled.split(',')

if (all.indexOf('*') !== -1) {
return false
Expand Down Expand Up @@ -97,7 +112,7 @@ module.exports = function (app) {
getDebugSettings: () => {
return { debugEnabled, rememberDebug }
},
rememberDebug: (enabled) => {
rememberDebug: (enabled: boolean) => {
if (debugPath) {
if (enabled) {
fs.writeFileSync(debugPath, debugEnabled)
Expand All @@ -115,7 +130,7 @@ module.exports = function (app) {
}
})
},
addDebug: (name) => {
addDebug: (name: string) => {
if (debugEnabled.length > 0) {
const all = debugEnabled.split(',')
if (all.indexOf(name) === -1) {
Expand All @@ -125,7 +140,7 @@ module.exports = function (app) {
enableDebug(name)
}
},
removeDebug: (name) => {
removeDebug: (name: string) => {
if (debugEnabled.length > 0) {
const all = debugEnabled.split(',')
const idx = all.indexOf(name)
Expand Down
6 changes: 2 additions & 4 deletions src/serverroutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
import { listAllSerialPorts } from './serialports'
import { StreamBundle } from './streambundle'
import { WithWrappedEmitter } from './events'
import { Logging } from './logging'
const readdir = util.promisify(fs.readdir)
const debug = createDebug('signalk-server:serverroutes')
import { getAISShipTypeName } from '@signalk/signalk-schema'
Expand All @@ -75,10 +76,7 @@ interface App
PluginManager,
WithWrappedEmitter {
webapps: Package[]
logging: {
rememberDebug: (r: boolean) => void
enableDebug: (r: string) => boolean
}
logging: Logging
activateSourcePriorities: () => void
streambundle: StreamBundle
}
Expand Down