Skip to content
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
22 changes: 18 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,30 @@ Right now the package uses `window.console` as log appender and produces the fol
[WARN] mail: it's just a warning, carry on { app: 'mail', uid: 'christoph' }
```

### Log level
The logger tries to detect the server configured logging level by default,
which can be configured using the `loglevel_frontend` option in the `config.php`.
In case no logging level was configured or detection failed, the logger will fallback to the *warning* level.

If the server is set to the debug mode the configured logging level will be set to the *debug* level.
In case no logging level was configured or detection failed, the logger will fallback to the *warning* level.
If the server is set to the debug mode the fallback will be the *debug* instead of the *warning* level.

Any message with a lower level than the configured will not be printed on the console.

You can override the logging level in both cases by setting it manually using the `setLogLevel` function
when building the logger.
#### Override the log level
You can override the logging level in both cases by setting it manually
using the `setLogLevel` function when building the logger.

It is also possible to debug an app without the need of manually recompile it to change the `setLogLevel`.
To do so the runtime debugging configuration can be changed by running this in the browser console:

```js
// debug a single app
window.__NC_LOGGER_DEBUG__=['YOUR_APP_ID']
// debug multiple apps
window.__NC_LOGGER_DEBUG__=['files', 'viewer']
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean there is no feature support like in the linked issue, but only per-app support?
So if you want to have a debug flag for a specific feature, you need to call it another app, correct?

Copy link
Contributor

@ShGKme ShGKme Nov 7, 2025

Choose a reason for hiding this comment

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

In the issue the proposal was to have an object with boolean flags. It's quite convenient because:

  1. You see the list of the flags that can be enabled/disabled
  2. You can just toggle true/false

in contrast, with the array of strings:

  1. You need to know the strings
  2. There are no "add"/"remove" operations in the array. Only via conversion to a Set.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, why make it a __BUILD_TIME_CONST__ or _private_variable_ like and not a public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically the idea is not to break existing loggers and need for refactoring your code base.
But instead allow to selectively enable debug logging for specific apps instead of all apps that would flood the logs.

Also, why make it a BUILD_TIME_CONST or private_variable like and not a public?

Reduce risks here to break anything as this is a global variable.
Alternative would be to move to OCA but then only OCA.Core would be kind of safe, as other name spaces could be used by an app (e.g. proposed OCA.Logger could be used by an app called logger).

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically the idea is not to break existing loggers and need for refactoring your code base.

Was @max-nextcloud proposal breaking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like you have to create a new logger that what I meant (as described below I might have a different use case)

```

This will enforce the *debug* logging level for the specified apps.

## Contributing

Expand Down
2 changes: 1 addition & 1 deletion REUSE.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ SPDX-PackageSupplier = "Nextcloud <info@nextcloud.com>"
SPDX-PackageDownloadLocation = "https://github.com/nextcloud-libraries/nextcloud-logger"

[[annotations]]
path = ["package-lock.json", "package.json", "tsconfig.json"]
path = ["package-lock.json", "package.json", "tsconfig.json", "tests/tsconfig.json"]
precedence = "aggregate"
SPDX-FileCopyrightText = "2019-2024 Nextcloud GmbH and Nextcloud contributors"
SPDX-License-Identifier = "GPL-3.0-or-later"
Expand Down
86 changes: 86 additions & 0 deletions lib/ALogger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-3.0-or-later
*/

import type { IContext, ILogger } from './contracts.ts'

import { LogLevel } from './contracts.ts'

/**
* Abstract base logger implementing common functionality
*/
export abstract class ALogger implements ILogger {
/**
* The initial logging context set by the constructor (LoggerBuilder factory)
*/
protected abstract context: IContext

/**
* Log a message with the specified level and context
*
* @param level - The log level requested
* @param message - The log message
* @param context - The logging context
*/
protected abstract log(level: LogLevel, message: string, context: IContext): void

public debug(message: string | Error, context?: IContext): void {
this.logIfNeeded(LogLevel.Debug, message, { ...context, ...this.context })
}

public info(message: string | Error, context?: IContext): void {
this.logIfNeeded(LogLevel.Info, message, { ...context, ...this.context })
}

public warn(message: string | Error, context?: IContext): void {
this.logIfNeeded(LogLevel.Warn, message, { ...context, ...this.context })
}

public error(message: string | Error, context?: IContext): void {
this.logIfNeeded(LogLevel.Error, message, { ...context, ...this.context })
}

public fatal(message: string | Error, context?: IContext): void {
this.logIfNeeded(LogLevel.Fatal, message, { ...context, ...this.context })
}

/**
* Check if the message should be logged and prepare the context
*
* @param level - The logging level requested
* @param message - The log message or Error object
* @param context - The logging context
*/
private logIfNeeded(level: LogLevel, message: string | Error, context: IContext): void {
// Skip if level is configured and this is below the level
if (typeof this.context?.level === 'number' && level < this.context?.level && !this.debuggingEnabled()) {
return
}

// Handle logging when only an error was passed as log message
if (typeof message === 'object') {
if (context.error) {
context.error = [context.error, message]
} else {
context.error = message
}
if (level === LogLevel.Debug || this.debuggingEnabled()) {
context.stacktrace = message.stack
}
return this.log(level, `${message.name}: ${message.message}`, context)
}

this.log(level, message, context)
}

/**
* Check if debugging is enabled for the current app
*/
private debuggingEnabled(): boolean {
const debugContexts = window.__NC_LOGGER_DEBUG__
return typeof this.context.app === 'string'
&& Array.isArray(debugContexts)
&& debugContexts.includes(this.context.app)
}
}
74 changes: 23 additions & 51 deletions lib/ConsoleLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,51 +5,19 @@

import type { IContext, ILogger } from './contracts.ts'

import { ALogger } from './ALogger.ts'
import { LogLevel } from './contracts.ts'

/* eslint-disable no-console -- This class is a console logger so it needs to write to the console. */
export class ConsoleLogger implements ILogger {
private context: IContext
export class ConsoleLogger extends ALogger implements ILogger {
protected context: IContext

constructor(context?: IContext) {
super()
this.context = context || {}
}

private formatMessage(message: string | Error, level: LogLevel, context?: IContext): string {
let msg = '[' + LogLevel[level].toUpperCase() + '] '

if (context && context.app) {
msg += context.app + ': '
}

if (typeof message === 'string') {
return msg + message
}

// basic error formatting
msg += `Unexpected ${message.name}`
if (message.message) {
msg += ` "${message.message}"`
}
// only add stack trace when debugging
if (level === LogLevel.Debug && message.stack) {
msg += `\n\nStack trace:\n${message.stack}`
}

return msg
}

log(level: LogLevel, message: string | Error, context: IContext) {
// Skip if level is configured and this is below the level
if (typeof this.context?.level === 'number' && level < this.context?.level) {
return
}

// Add error object to context
if (typeof message === 'object' && context?.error === undefined) {
context.error = message
}

protected log(level: LogLevel, message: string | Error, context: IContext) {
switch (level) {
case LogLevel.Debug:
console.debug(this.formatMessage(message, LogLevel.Debug, context), context)
Expand All @@ -70,24 +38,28 @@ export class ConsoleLogger implements ILogger {
}
}

debug(message: string | Error, context?: IContext): void {
this.log(LogLevel.Debug, message, { ...this.context, ...context })
}
private formatMessage(message: string | Error, level: LogLevel, context?: IContext): string {
let msg = '[' + LogLevel[level].toUpperCase() + '] '

info(message: string | Error, context?: IContext): void {
this.log(LogLevel.Info, message, { ...this.context, ...context })
}
if (context && context.app) {
msg += context.app + ': '
}

warn(message: string | Error, context?: IContext): void {
this.log(LogLevel.Warn, message, { ...this.context, ...context })
}
if (typeof message === 'string') {
return msg + message
}

error(message: string | Error, context?: IContext): void {
this.log(LogLevel.Error, message, { ...this.context, ...context })
}
// basic error formatting
msg += `Unexpected ${message.name}`
if (message.message) {
msg += ` "${message.message}"`
}
// only add stack trace when debugging
if (level === LogLevel.Debug && message.stack) {
msg += `\n\nStack trace:\n${message.stack}`
}

fatal(message: string | Error, context?: IContext): void {
this.log(LogLevel.Fatal, message, { ...this.context, ...context })
return msg
}
}

Expand Down
1 change: 1 addition & 0 deletions lib/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* SPDX-FileCopyrightText: 2019-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-3.0-or-later
*/

export enum LogLevel {
Debug = 0,
Info = 1,
Expand Down
2 changes: 2 additions & 0 deletions lib/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import type { LogLevel } from './contracts.ts'

declare global {
interface Window {
__NC_LOGGER_DEBUG__?: string[]

_oc_config: {
loglevel: LogLevel
}
Expand Down
45 changes: 29 additions & 16 deletions tests/ConsoleLogger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@
* SPDX-FileCopyrightText: 2023-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-3.0-or-later
*/
import { afterEach, describe, expect, it, test, vi } from 'vitest'
import { ConsoleLogger, buildConsoleLogger } from '../lib/ConsoleLogger'

import { beforeEach, describe, expect, it, test, vi } from 'vitest'
import { buildConsoleLogger, ConsoleLogger } from '../lib/ConsoleLogger.ts'

// Dummy Error
class MyError extends Error {

constructor(msg: string) {
super(msg)
this.name = 'MyError'
}

}

afterEach(() => {
beforeEach(() => {
delete window.__NC_LOGGER_DEBUG__
vi.resetAllMocks()
})

Expand All @@ -33,8 +33,6 @@ test('building the console logger', () => {
})

describe('ConsoleLogger', () => {
afterEach(() => { vi.resetAllMocks() })

it('logs debug messages', () => {
const logger = new ConsoleLogger()
const debug = vi.spyOn(window.console, 'debug').mockImplementation(() => {})
Expand Down Expand Up @@ -94,10 +92,10 @@ describe('ConsoleLogger', () => {
const logger = new ConsoleLogger({ one: 1, two: 2 })
const debug = vi.spyOn(window.console, 'debug').mockImplementation(() => {})

logger.debug('Should be logged', { two: 3 })
logger.debug('Should be logged', { three: 3 })
expect(debug).toHaveBeenCalledTimes(1)
expect(debug.mock.calls[0][0]).toBe('[DEBUG] Should be logged')
expect(debug.mock.calls[0][1]).toEqual({ one: 1, two: 3 })
expect(debug.mock.calls[0][1]).toEqual({ one: 1, two: 2, three: 3 })
})

it('allows extending empty global context', () => {
Expand All @@ -119,6 +117,20 @@ describe('ConsoleLogger', () => {
expect(debug).toHaveBeenCalledTimes(0)
})

it('respects the runtime debug configuration', () => {
const logger = new ConsoleLogger({ app: 'test', level: 2 })

const debug = vi.spyOn(window.console, 'debug')
debug.mockImplementationOnce(() => {})

logger.debug('Should not be logged')
expect(debug).toHaveBeenCalledTimes(0)

window.__NC_LOGGER_DEBUG__ = ['files', 'test']
logger.debug('Should be logged now')
expect(debug).toHaveBeenCalledTimes(1)
})

it('logs Error objects', () => {
const error = new MyError('some message')
const logger = new ConsoleLogger({})
Expand All @@ -128,10 +140,9 @@ describe('ConsoleLogger', () => {

logger.warn(error)
expect(warn).toHaveBeenCalledTimes(1)
expect(console[0][0]).toContain('MyError')
expect(console[0][0]).toContain('some message')
expect(console[0][0]).not.toContain('Stack trace')
expect(console[0][0]).toMatch('MyError: some message')
expect(console[0][1]).toHaveProperty('error', error)
expect(console[0][1]).not.toHaveProperty('stacktrace', error.stack)
})

it('logs Error objects and stack trace on debug', () => {
Expand All @@ -143,9 +154,9 @@ describe('ConsoleLogger', () => {

logger.debug(error)
expect(debug).toHaveBeenCalledTimes(1)
expect(console[0][0]).toContain('MyError')
expect(console[0][0]).toContain('some message')
expect(console[0][0]).toContain('Stack trace:')
expect(console[0][0]).toContain('MyError: some message')
expect(console[0][1]).toHaveProperty('error', error)
expect(console[0][1]).toHaveProperty('stacktrace', error.stack)
})

it('logs Error objects and does not override context', () => {
Expand All @@ -159,6 +170,8 @@ describe('ConsoleLogger', () => {
expect(warn).toHaveBeenCalledTimes(1)
expect(console[0][0]).toContain('MyError')
expect(console[0][0]).toContain('some message')
expect(console[0][1]).toHaveProperty('error', 'none')
expect(console[0][1]).toHaveProperty('error')
// @ts-expect-error - We know error is an array here
expect(console[0][1]!.error).toEqual(['none', error])
})
})
4 changes: 4 additions & 0 deletions tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"extends": "../tsconfig.json",
"include": ["../lib", "."]
}