Skip to content

Commit

Permalink
fix: render (log) errors (#4439)
Browse files Browse the repository at this point in the history
* fix: log errors

if just the error was passed to the log entry it was not logged
previously. This might have swallowed up quite a few errors in the past.

Most of the time the error is formated into the `msg` on the log entry
so I added logic that checks whether the entry already includes the
the error and if so doesn't duplicate it.

* feat: log dividers with terminal width

* test: fix assertion

The error detail is written to the error log file as it proved to be
too noisy. That's why the error renderer was removed from the renderers
in the first place.

---

Co-authored-by: Vladimir Vagaytsev <[email protected]>
  • Loading branch information
Orzelius and vvagaytsev authored May 31, 2023
1 parent 4dc6735 commit 4fe827f
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
14 changes: 4 additions & 10 deletions core/src/cli/command-line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import { findProjectConfig } from "../config/base"
import { toGardenError } from "../exceptions"
import { Garden } from "../garden"
import type { Log } from "../logger/log-entry"
import { getRootLogger } from "../logger/logger"
import { renderDivider } from "../logger/util"
import { getTermWidth, renderDivider } from "../logger/util"
import { getGardenForRequest } from "../server/commands"
import type { GardenInstanceManager } from "../server/instance-manager"
import { TypedEventEmitter } from "../util/events"
Expand Down Expand Up @@ -94,7 +93,7 @@ export function logCommandSuccess({ commandName, log, width }: { commandName: st
}

export function logCommandOutputErrors({ errors, log, width }: { errors: Error[]; log: Log; width: number }) {
renderCommandErrors(getRootLogger(), errors, log)
renderCommandErrors(log.root, errors, log)
log.error({ msg: renderDivider({ width, color: chalk.red }) })
}

Expand Down Expand Up @@ -457,15 +456,10 @@ export class CommandLine extends TypedEventEmitter<CommandLineEvents> {
this.renderCommandLine()
}

getTermWidth() {
// TODO: accept stdout in constructor
return process.stdout?.columns || 100
}

private printWithDividers(text: string, title: string) {
let width = max(text.split("\n").map((l) => stringWidth(l.trimEnd()))) || 0
width += 2
const termWidth = this.getTermWidth()
const termWidth = getTermWidth()
const minWidth = stringWidth(title) + 10

if (width > termWidth) {
Expand Down Expand Up @@ -668,7 +662,7 @@ ${chalk.white.underline("Keys:")}
opts: ParameterValues<any>
}) {
const id = uuidv4()
const width = this.getTermWidth() - 2
const width = getTermWidth() - 2

const prepareParams = {
log: this.log,
Expand Down
1 change: 0 additions & 1 deletion core/src/cli/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,6 @@ export function renderCommandErrors(logger: Logger, errors: Error[], log?: Log)

for (const error of gardenErrors) {
errorLog.error({
msg: error.message,
error,
})
// Output error details to console when log level is silly
Expand Down
19 changes: 10 additions & 9 deletions core/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@
import logSymbols from "log-symbols"
import chalk from "chalk"
import stripAnsi from "strip-ansi"
import { isArray, repeat } from "lodash"
import { isArray, repeat, trim } from "lodash"
import stringWidth = require("string-width")
import hasAnsi = require("has-ansi")
import format from "date-fns/format"

import { LogEntry } from "./log-entry"
import { JsonLogEntry } from "./writers/json-terminal-writer"
import { highlightYaml, safeDumpYaml } from "../util/serialization"
Expand Down Expand Up @@ -45,15 +43,17 @@ export function combineRenders(entry: LogEntry, logger: Logger, renderers: Rende
}

export function renderError(entry: LogEntry): string {
const { msg, error } = entry
const { error, msg } = entry

let out = ""

if (error) {
if (msg) {
out += "\n\n"
const noAnsiErr = stripAnsi(error.message || "")
const noAnsiMsg = stripAnsi(msg || "")
// render error only if message doesn't already contain it
if (!noAnsiMsg?.includes(trim(noAnsiErr, "\n"))) {
out = "\n\n" + chalk.red(error.message)
}
out += formatGardenErrorWithDetail(toGardenError(error))
}

return out
Expand Down Expand Up @@ -158,8 +158,8 @@ export function renderSection(entry: LogEntry): string {
* Formats entries for the terminal writer.
*/
export function formatForTerminal(entry: LogEntry, logger: Logger): string {
const { msg: msg, symbol, data } = entry
const empty = [msg, symbol, data].every((val) => val === undefined)
const { msg: msg, symbol, data, error } = entry
const empty = [msg, symbol, data, error].every((val) => val === undefined)

if (empty) {
return ""
Expand All @@ -170,6 +170,7 @@ export function formatForTerminal(entry: LogEntry, logger: Logger): string {
renderSymbol,
renderSection,
renderMsg,
renderError,
renderData,
() => "\n",
])
Expand Down
10 changes: 9 additions & 1 deletion core/src/logger/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,17 @@ const getNumberOfCharsPerWidth = (char: string, width: number) => width / string

// Adapted from https://github.com/JureSotosek/ink-divider
export function renderDivider({
width = 80,
width = undefined,
char = "─",
titlePadding = 1,
color,
title,
padding = 0,
}: DividerOpts = {}) {
const pad = " "
if (!width) {
width = getTermWidth()
}

if (!color) {
color = chalk.white
Expand All @@ -124,6 +127,11 @@ export function renderDivider({
return paddingString + dividerSideString + titleString + dividerSideString + paddingString
}

export const getTermWidth = () => {
// TODO: accept stdout as param
return process.stdout?.columns || 100
}

export function renderDuration(duration: number): string {
return `(took ${duration} sec)`
}
Expand Down
3 changes: 1 addition & 2 deletions core/test/unit/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ describe("renderers", () => {
}
const log = logger.createLog().info({ msg: "foo", error })
const rendered = renderError(log.entries[0])
expect(rendered).to.include("Error: hello error")
expect(rendered).to.include("Error Details:")
expect(rendered).to.include("hello error")
})
})
describe("renderSection", () => {
Expand Down

0 comments on commit 4fe827f

Please sign in to comment.