mirror of
https://github.com/desktop/desktop
synced 2024-09-19 08:02:22 +00:00
Improve git error API to show the combined stdout and stderr outputs
Co-Authored-By: Markus Olsson <634063+niik@users.noreply.github.com>
This commit is contained in:
parent
43c7e3acde
commit
ecdb58b3b3
|
@ -15,6 +15,9 @@ import * as Path from 'path'
|
||||||
import { Repository } from '../../models/repository'
|
import { Repository } from '../../models/repository'
|
||||||
import { getConfigValue, getGlobalConfigValue } from './config'
|
import { getConfigValue, getGlobalConfigValue } from './config'
|
||||||
import { isErrnoException } from '../errno-exception'
|
import { isErrnoException } from '../errno-exception'
|
||||||
|
import { ChildProcess } from 'child_process'
|
||||||
|
import byline from 'byline'
|
||||||
|
import { Readable } from 'stream'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* An extension of the execution options in dugite that
|
* An extension of the execution options in dugite that
|
||||||
|
@ -54,6 +57,9 @@ export interface IGitResult extends DugiteResult {
|
||||||
/** The human-readable error description, based on `gitError`. */
|
/** The human-readable error description, based on `gitError`. */
|
||||||
readonly gitErrorDescription: string | null
|
readonly gitErrorDescription: string | null
|
||||||
|
|
||||||
|
/** Both stdout and stderr combined. */
|
||||||
|
readonly combinedOutput: string
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The path that the Git command was executed from, i.e. the
|
* The path that the Git command was executed from, i.e. the
|
||||||
* process working directory (not to be confused with the Git
|
* process working directory (not to be confused with the Git
|
||||||
|
@ -61,22 +67,6 @@ export interface IGitResult extends DugiteResult {
|
||||||
*/
|
*/
|
||||||
readonly path: string
|
readonly path: string
|
||||||
}
|
}
|
||||||
|
|
||||||
function getResultMessage(result: IGitResult) {
|
|
||||||
const description = result.gitErrorDescription
|
|
||||||
if (description) {
|
|
||||||
return description
|
|
||||||
}
|
|
||||||
|
|
||||||
if (result.stderr.length) {
|
|
||||||
return result.stderr
|
|
||||||
} else if (result.stdout.length) {
|
|
||||||
return result.stdout
|
|
||||||
} else {
|
|
||||||
return 'Unknown error'
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
export class GitError extends Error {
|
export class GitError extends Error {
|
||||||
/** The result from the failed command. */
|
/** The result from the failed command. */
|
||||||
public readonly result: IGitResult
|
public readonly result: IGitResult
|
||||||
|
@ -84,12 +74,35 @@ export class GitError extends Error {
|
||||||
/** The args for the failed command. */
|
/** The args for the failed command. */
|
||||||
public readonly args: ReadonlyArray<string>
|
public readonly args: ReadonlyArray<string>
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether or not the error message is just the raw output of the git command.
|
||||||
|
*/
|
||||||
|
public readonly isRawMessage: boolean
|
||||||
|
|
||||||
public constructor(result: IGitResult, args: ReadonlyArray<string>) {
|
public constructor(result: IGitResult, args: ReadonlyArray<string>) {
|
||||||
super(getResultMessage(result))
|
let rawMessage = true
|
||||||
|
let message
|
||||||
|
|
||||||
|
if (result.gitErrorDescription) {
|
||||||
|
message = result.gitErrorDescription
|
||||||
|
rawMessage = false
|
||||||
|
} else if (result.combinedOutput.length > 0) {
|
||||||
|
message = result.combinedOutput
|
||||||
|
} else if (result.stderr.length) {
|
||||||
|
message = result.stderr
|
||||||
|
} else if (result.stdout.length) {
|
||||||
|
message = result.stdout
|
||||||
|
} else {
|
||||||
|
message = 'Unknown error'
|
||||||
|
rawMessage = false
|
||||||
|
}
|
||||||
|
|
||||||
|
super(message)
|
||||||
|
|
||||||
this.name = 'GitError'
|
this.name = 'GitError'
|
||||||
this.result = result
|
this.result = result
|
||||||
this.args = args
|
this.args = args
|
||||||
|
this.isRawMessage = rawMessage
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -123,8 +136,24 @@ export async function git(
|
||||||
expectedErrors: new Set(),
|
expectedErrors: new Set(),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let combinedOutput = ''
|
||||||
const opts = { ...defaultOptions, ...options }
|
const opts = { ...defaultOptions, ...options }
|
||||||
|
|
||||||
|
opts.processCallback = (process: ChildProcess) => {
|
||||||
|
options?.processCallback?.(process)
|
||||||
|
|
||||||
|
const combineOutput = (readable: Readable | null) => {
|
||||||
|
if (readable) {
|
||||||
|
byline(readable).on('data', (line: string) => {
|
||||||
|
combinedOutput += line + '\n'
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
combineOutput(process.stderr)
|
||||||
|
combineOutput(process.stdout)
|
||||||
|
}
|
||||||
|
|
||||||
// Explicitly set TERM to 'dumb' so that if Desktop was launched
|
// Explicitly set TERM to 'dumb' so that if Desktop was launched
|
||||||
// from a terminal or if the system environment variables
|
// from a terminal or if the system environment variables
|
||||||
// have TERM set Git won't consider us as a smart terminal.
|
// have TERM set Git won't consider us as a smart terminal.
|
||||||
|
@ -160,7 +189,13 @@ export async function git(
|
||||||
}
|
}
|
||||||
|
|
||||||
const gitErrorDescription = gitError ? getDescriptionForError(gitError) : null
|
const gitErrorDescription = gitError ? getDescriptionForError(gitError) : null
|
||||||
const gitResult = { ...result, gitError, gitErrorDescription, path }
|
const gitResult = {
|
||||||
|
...result,
|
||||||
|
gitError,
|
||||||
|
gitErrorDescription,
|
||||||
|
combinedOutput,
|
||||||
|
path,
|
||||||
|
}
|
||||||
|
|
||||||
let acceptableError = true
|
let acceptableError = true
|
||||||
if (gitError && opts.expectedErrors) {
|
if (gitError && opts.expectedErrors) {
|
||||||
|
|
|
@ -111,10 +111,9 @@ export class AppError extends React.Component<IAppErrorProps, IAppErrorState> {
|
||||||
const e = error instanceof ErrorWithMetadata ? error.underlyingError : error
|
const e = error instanceof ErrorWithMetadata ? error.underlyingError : error
|
||||||
|
|
||||||
if (e instanceof GitError) {
|
if (e instanceof GitError) {
|
||||||
// See getResultMessage in core.ts
|
// If the error message is just the raw git output, display it in
|
||||||
// If the error message is the same as stderr or stdout then we know
|
// fixed-width font
|
||||||
// it's output from git and we'll display it in fixed-width font
|
if (e.isRawMessage) {
|
||||||
if (e.message === e.result.stderr || e.message === e.result.stdout) {
|
|
||||||
const formattedMessage = this.formatGitErrorMessage(e.message)
|
const formattedMessage = this.formatGitErrorMessage(e.message)
|
||||||
return <p className="monospace">{formattedMessage}</p>
|
return <p className="monospace">{formattedMessage}</p>
|
||||||
}
|
}
|
||||||
|
|
|
@ -112,6 +112,7 @@ describe('git/core', () => {
|
||||||
gitErrorDescription: null,
|
gitErrorDescription: null,
|
||||||
stderr,
|
stderr,
|
||||||
stdout: '',
|
stdout: '',
|
||||||
|
combinedOutput: stderr,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue