feat(logger): persistent file logging and structured API errors - #1082
feat(logger): persistent file logging and structured API errors#1082pdesoyres-cc wants to merge 12 commits into
Conversation
|
🔎 A preview has been automatically published! If you created the alias to the preview script, you can run this command to download and install this preview: clever-preview update enhance-loggerYou can also run it from your local repository: ./scripts/preview.js update enhance-logger
This preview will be deleted once this PR is closed. |
28ba915 to
dd531cf
Compare
3b6b530 to
79e36b8
Compare
Move API error construction out of the logger and into the send-to-api layer so failures propagate as real Error subclasses. The logger no longer needs to know the shape of API response bodies, and the full response body is preserved on the error for callers that need it.
Mirror the writeStderr helper so stdout and stderr go through the same path, and make Logger output easier to stub in tests.
Adopting @clevercloud/scribe pulled in pino → thread-stream, whose CJS shape makes @rollup/plugin-commonjs emit virtual `package.json?commonjs-proxy` modules. The preview-version transform matched those proxies and tried to JSON.parse JavaScript, failing `scripts/bundle-cjs.js` and the downstream binary compilation.
Add `Logger.printWarning` for terminal-facing messages (yellow ⚠ prefix on stdout) and migrate the four call sites that were using `Logger.warn` for that purpose. The scribe migration in progress on this branch repurposes `warn` for structured logging, so user-facing CLI warnings need their own channel.
The two call sites converted here aren't logging exceptions — they print a usage check (curl) and a post-failure hint (k8s) to stderr before returning. Routing them through `Logger.error` added an `[ERROR]` prefix and, in verbose mode, stacktrace output that wasn't relevant. Mirrors the warn / printWarning split from 6040158: the scribe migration in progress on this branch repurposes `error` for structured logging, so user-facing CLI errors need their own channel.
…intln Finishes the console.log migration started in 0b6f00e so every stdout write goes through the Logger facade — a prerequisite for swapping the backend to scribe without leaving stray unstructured prints behind. Also drops a leftover debug log of the SSH key API responseBody id that was only useful while narrowing down the 505 error case.
Continues the migration to centralize console output behind the Logger so the underlying backend can be swapped without touching command code. Adds a thin printTable helper and updates every direct console.table call site to use it.
…ation The recent refactor series routed every console call in bin/ and src/ through the Logger abstraction. Add no-console to prevent regressions and keep all CLI output going through a single sink. Scripts stay exempt since release tooling has no Logger and prints directly. The Logger's own console.table sink keeps an inline disable as the sanctioned escape hatch.
Adds @clevercloud/scribe so every Logger call is written to a per-OS log file (Windows LOCALAPPDATA, macOS ~/Library/Logs, Linux XDG_STATE_HOME) with size-based rotation. Gives users and support a durable trace even when the CLI is run non-verbose, where stderr is intentionally kept minimal. The console renderer (renamed prettyLog) now gates on IS_VERBOSE for every severity, warn included — the file already captures it, so there is no reason to leak it to stderr by default. Also guards the stacktrace branch on `error instanceof Error` so string-only errors no longer try to dump a non-existent stack.
The error branch duplicated styling, stacktrace handling, and stderr routing that the other severities did not have. Folding error into prettyLog with a SEVERITY_STYLES map collapses both paths into one, so styled prefixes, API-error processing, and stream selection are decided in a single place — and warn/info/debug pick up the same styled prefix treatment along the way.
Scribe's file transport is backed by a Pino worker thread, so anything still in its async buffer is lost when the CLI exits via a synchronous `process.exit()`. We now route every exit through a small helper that awaits `shutdown()` first, covering command success and failure (via the cliparse wrapper), EPIPE on stdout, SIGINT/SIGTERM, prompt cancellation, ssh subprocess teardown, and curl/config validation errors. Cliparse's own parse-time `process.exit(1)` calls are untouched — they fire before any meaningful logging, so the lost file lines aren't worth a global `process.exit` shim.
The previous commit routed every exit through `src/lib/exit.js` so log buffers get flushed before the process dies. Without a lint guard, future code will quietly reintroduce direct `process.exit()` calls and silently drop log lines again. The rule keeps the helper as the single sanctioned exit point.
79e36b8 to
dbb4234
Compare
|
@pdesoyres-cc I rebased to ease the review and tests |
|
Tested 1. The generated CJS bundle and macOS ARM64 binary fail to start. 2. A handled error can exit 0. 3. Crash when the log directory isn't writable. 4. 5. master prints blank lines there. 6. The deprecation warning is missing. 7. Errors that used to be matched on
8. Every row of the output I got carried one, including The eight issues were also reproduced from separate source snapshots with fresh |
3 participants
Context
The previous
Loggeronly wrote to stdout, so output vanished as soon as the process exited — bugreports were impossible to triage after the fact. API errors went through an ad-hoc
processErrorthat rendered a
fieldsmap, hid the raw response body, and made support tickets painful. A longtail of direct
console.*calls in commands meantCLEVER_QUIET/CLEVER_VERBOSEappliedunevenly across the CLI.
Changes
@clevercloud/scribeand persist every log level to a rotating file at OS-conventionalpaths:
$XDG_STATE_HOME/clever-cloud/(Linux),~/Library/Logs/clever-cloud/(macOS),%LOCALAPPDATA%\clever-cloud\Logs\(Windows). Rotation kicks in around 250k, capped at 50 files.ApiError extends Errorexposingcode,status, rawbody,url,headers. Thelegacy
processErrorfieldsrendering is gone —ApiError.bodycarries the raw response.prettyLoghandles every severity, including stack tracesunder
CLEVER_VERBOSE=1and styled[ERROR]/[WARN]/[INFO]/[DEBUG]prefixes.console.log/console.table/console.error/ stderr writesthrough
Logger.println/printTable/printErrorLine/warn, and split user-facingwarnings from the internal
warnchannel.src/lib/exit.jshelper that awaits scribe'sshutdown()beforecalling
process.exit, so the file transport's async buffer is flushed on success, failure,EPIPE, SIGINT/SIGTERM, prompt cancellation, and ssh/curl/config error paths.
bin/*.jsandsrc/**/*.js:no-consoleto lock in the Loggermigration, and a
no-restricted-propertiesrule forbiddingprocess.exitoutside the newexit helper.
Implementation notes
The pretty-print path was rewritten rather than patched: the old code had divergent branches for
error(red prefix, stack trace, stderr) and the other severities (no styling, stdout, gated onIS_VERBOSE). Routing every severity throughprettyLogmakes theIS_QUIET/IS_VERBOSEandstderr-vs-stdout decisions explicit instead of scattered.
Cliparse's own parse-time
process.exit(1)calls are deliberately left untouched — they firebefore any meaningful logging happens, so the lost file lines aren't worth a global
process.exitshim. The ESLint rule's
ignoresentry exemptssrc/lib/exit.jsitself, which is the onesanctioned caller of
process.exit.How to review
src/logger.js—prettyLogandgetLogFilePathare the load-bearing changes.src/lib/api-error.jsplustoApiError/parseError*insrc/models/send-to-api.jsto see how API failures now flow to the user.
src/lib/exit.jsand the call sites inbin/clever.js,src/lib/cliparse-patched.js,src/lib/prompts.js, and the ssh/curl/config commands to confirm every exit path flushes.clevercommand and confirm a log file appears under the OS-specific path above; forcea few rotations to check the 250k / 50-files cap.
CLEVER_QUIET=1suppresses stdout but the file is still written;CLEVER_VERBOSE=1surfaces[DEBUG]/[INFO]entries on stdout and prints full stack traces for thrownErrors.<message> [<code>]and thatApiError.bodycarries the raw response payload.buffer was flushed before exit).