Skip to content

Refactor error thrown when a git command fails #622

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

Merged
merged 1 commit into from
Feb 21, 2023
Merged
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
4 changes: 4 additions & 0 deletions lib/git.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@
require 'git/base'
require 'git/branch'
require 'git/branches'
require 'git/command_line_result'
require 'git/config'
require 'git/diff'
require 'git/encoding_utils'
require 'git/escaped_path'
require 'git/failed_error'
require 'git/git_execute_error'
require 'git/index'
require 'git/lib'
require 'git/log'
require 'git/object'
require 'git/path'
require 'git/remote'
require 'git/repository'
require 'git/signaled_error'
require 'git/status'
require 'git/stash'
require 'git/stashes'
Expand Down
86 changes: 86 additions & 0 deletions lib/git/command_line_result.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# frozen_string_literal: true

module Git
# The result of running a git command
#
# This object stores the Git command executed and its status, stdout, and stderr.
#
# @api public
#
class CommandLineResult
# Create a CommandLineResult object
#
# @example
# `true`
# git_cmd = %w[git version]
# status = $?
# stdout = "git version 2.39.1\n"
# stderr = ""
# result = Git::CommandLineResult.new(git_cmd, status, stdout, stderr)
#
# @param git_cmd [Array<String>] the git command that was executed
# @param status [Process::Status] the status of the process
# @param stdout [String] the output of the process
# @param stderr [String] the error output of the process
#
def initialize(git_cmd, status, stdout, stderr)
@git_cmd = git_cmd
@status = status
@stdout = stdout
@stderr = stderr
end

# @attribute [r] git_cmd
#
# The git command that was executed
#
# @example
# git_cmd = %w[git version]
# result = Git::CommandLineResult.new(git_cmd, $?, "", "")
# result.git_cmd #=> ["git", "version"]
#
# @return [Array<String>]
#
attr_reader :git_cmd

# @attribute [r] status
#
# The status of the process
#
# @example
# `true`
# status = $?
# result = Git::CommandLineResult.new(status, "", "")
# result.status #=> #<Process::Status: pid 87859 exit 0>
#
# @return [Process::Status]
#
attr_reader :status

# @attribute [r] stdout
#
# The output of the process
#
# @example
# stdout = "git version 2.39.1\n"
# result = Git::CommandLineResult.new($?, stdout, "")
# result.stdout #=> "git version 2.39.1\n"
#
# @return [String]
#
attr_reader :stdout

# @attribute [r] stderr
#
# The error output of the process
#
# @example
# stderr = "Tag not found\n"
# result = Git::CommandLineResult.new($?, "", stderr)
# result.stderr #=> "Tag not found\n"
#
# @return [String]
#
attr_reader :stderr
end
end
51 changes: 51 additions & 0 deletions lib/git/failed_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

require 'git/git_execute_error'

module Git
# This error is raised when a git command fails
#
# The git command executed, status, stdout, and stderr are available from this
# object. The #message includes the git command, the status of the process, and
# the stderr of the process.
#
# @api public
#
class FailedError < Git::GitExecuteError
# Create a FailedError object
#
# @example
# `exit 1` # set $? appropriately for this example
# result = Git::CommandLineResult.new(%w[git status], $?, '', "failed")
# error = Git::FailedError.new(result)
# error.message #=>
# "[\"git\", \"status\"]\nstatus: pid 89784 exit 1\nstderr: \"failed\""
#
# @param result [Git::CommandLineResult] the result of the git command including
# the git command, status, stdout, and stderr
#
def initialize(result)
super("#{result.git_cmd}\nstatus: #{result.status}\nstderr: #{result.stderr.inspect}")

Choose a reason for hiding this comment

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

@jcouball I think stdout also has to be printed here since stderr is redirected to stdout??

git_cmd = "#{Git::Base.config.binary_path} #{global_opts} #{escaped_cmd} #{redirect} 2>&1"

@result = result
end

# @attribute [r] result
#
# The result of the git command including the git command and its status and output
#
# @example
# `exit 1` # set $? appropriately for this example
# result = Git::CommandLineResult.new(%w[git status], $?, '', "failed")
# error = Git::FailedError.new(result)
# error.result #=>
# #<Git::CommandLineResult:0x00000001046bd488
# @git_cmd=["git", "status"],
# @status=#<Process::Status: pid 89784 exit 1>,
# @stderr="failed",
# @stdout="">
#
# @return [Git::CommandLineResult]
#
attr_reader :result
end
end
7 changes: 7 additions & 0 deletions lib/git/git_execute_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module Git
# This error is raised when a git command fails
#
class GitExecuteError < StandardError; end
end
15 changes: 7 additions & 8 deletions lib/git/lib.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
require 'git/failed_error'
require 'logger'
require 'tempfile'
require 'zlib'

module Git

class GitExecuteError < StandardError
end

class Lib

@@semaphore = Mutex.new
Expand Down Expand Up @@ -1128,21 +1125,23 @@ def command(*cmd, redirect: '', chomp: true, &block)

command_thread = nil;

exitstatus = nil
status = nil

with_custom_env_variables do
command_thread = Thread.new do
output = run_command(git_cmd, &block)
exitstatus = $?.exitstatus
status = $?
end
command_thread.join
end

@logger.info(git_cmd)
@logger.debug(output)

raise Git::GitExecuteError, "#{git_cmd}:#{output}" if
exitstatus > 1 || (exitstatus == 1 && output != '')
if status.exitstatus > 1 || (status.exitstatus == 1 && output != '')
result = Git::CommandLineResult.new(git_cmd, status, output, '')
raise Git::FailedError.new(result)
end

output.chomp! if output && chomp && !block_given?

Expand Down
50 changes: 50 additions & 0 deletions lib/git/signaled_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require 'git/git_execute_error'

module Git
# This error is raised when a git command exits because of an uncaught signal
#
# The git command executed, status, stdout, and stderr are available from this
# object. The #message includes the git command, the status of the process, and
# the stderr of the process.
#
# @api public
#
class SignaledError < Git::GitExecuteError
# Create a SignaledError object
#
# @example
# `kill -9 $$` # set $? appropriately for this example
# result = Git::CommandLineResult.new(%w[git status], $?, '', "killed")
# error = Git::SignaledError.new(result)
# error.message #=>
# "[\"git\", \"status\"]\nstatus: pid 88811 SIGKILL (signal 9)\nstderr: \"killed\""
#
# @param result [Git::CommandLineResult] the result of the git command including the git command, status, stdout, and stderr
#
def initialize(result)
super("#{result.git_cmd}\nstatus: #{result.status}\nstderr: #{result.stderr.inspect}")
@result = result
end

# @attribute [r] result
#
# The result of the git command including the git command, status, and output
#
# @example
# `kill -9 $$` # set $? appropriately for this example
# result = Git::CommandLineResult.new(%w[git status], $?, '', "killed")
# error = Git::SignaledError.new(result)
# error.result #=>
# #<Git::CommandLineResult:0x000000010470f6e8
# @git_cmd=["git", "status"],
# @status=#<Process::Status: pid 88811 SIGKILL (signal 9)>,
# @stderr="killed",
# @stdout="">
#
# @return [Git::CommandLineResult]
#
attr_reader :result
end
end
2 changes: 1 addition & 1 deletion tests/units/test_branch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_branch_create_and_switch
assert(git.status.untracked.assoc('test-file1'))
assert(!git.status.added.assoc('test-file1'))

assert_raise Git::GitExecuteError do
assert_raise Git::FailedError do
git.branch('new_branch').delete
end
assert_equal(1, git.branches.select { |b| b.name == 'new_branch' }.size)
Expand Down
17 changes: 17 additions & 0 deletions tests/units/test_command_line_result.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require 'test_helper'

class TestCommamndLineResult < Test::Unit::TestCase
def test_initialization
git_cmd = Object.new
status = Object.new
stdout = Object.new
stderr = Object.new

result = Git::CommandLineResult.new(git_cmd, status, stdout, stderr)

assert_equal(git_cmd, result.git_cmd)
assert_equal(status, result.status)
assert_equal(stdout, result.stdout)
assert_equal(stderr, result.stderr)
end
end
2 changes: 1 addition & 1 deletion tests/units/test_commit_with_empty_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def setup
def test_without_allow_empty_message_option
Dir.mktmpdir do |dir|
git = Git.init(dir)
assert_raises Git::GitExecuteError do
assert_raises Git::FailedError do
git.commit('', { allow_empty: true })
end
end
Expand Down
23 changes: 23 additions & 0 deletions tests/units/test_failed_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
require 'test_helper'

class TestFailedError < Test::Unit::TestCase
def test_initializer
status = Struct.new(:to_s).new('pid 89784 exit 1')
result = Git::CommandLineResult.new(%w[git status], status, '', "failed")

error = Git::FailedError.new(result)

assert(error.is_a?(Git::GitExecuteError))
assert_equal(result, error.result)
end

def test_message
status = Struct.new(:to_s).new('pid 89784 exit 1')
result = Git::CommandLineResult.new(%w[git status], status, '', "failed")

error = Git::FailedError.new(result)

expected_message = "[\"git\", \"status\"]\nstatus: pid 89784 exit 1\nstderr: \"failed\""
assert_equal(expected_message, error.message)
end
end
7 changes: 7 additions & 0 deletions tests/units/test_git_execute_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
require 'test_helper'

class TestGitExecuteError < Test::Unit::TestCase
def test_is_a_standard_error
assert(Git::GitExecuteError < StandardError)
end
end
2 changes: 1 addition & 1 deletion tests/units/test_lib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_commit_with_no_verify
@lib.add('test_file_2')

# Error raised because of pre-commit hook and no use of no_verify option
assert_raise Git::GitExecuteError do
assert_raise Git::FailedError do
@lib.commit('commit without no verify and pre-commit file')
end

Expand Down
2 changes: 1 addition & 1 deletion tests/units/test_log.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def test_get_log_path
end

def test_log_file_noexist
assert_raise Git::GitExecuteError do
assert_raise Git::FailedError do
@git.log.object('no-exist.txt').size
end
end
Expand Down
4 changes: 2 additions & 2 deletions tests/units/test_remotes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ def test_fetch_command_injection
origin = "--upload-pack=touch #{test_file};"
begin
git.fetch(origin, { ref: 'some/ref/head' })
rescue Git::GitExecuteError
rescue Git::FailedError
# This is expected
else
raise 'Expected Git::GitExecuteError to be raised'
raise 'Expected Git::Failed to be raised'
end

vulnerability_exists = File.exist?(test_file)
Expand Down
23 changes: 23 additions & 0 deletions tests/units/test_signaled_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
require 'test_helper'

class TestSignaledError < Test::Unit::TestCase
def test_initializer
status = Struct.new(:to_s).new('pid 65628 SIGKILL (signal 9)') # `kill -9 $$`
result = Git::CommandLineResult.new(%w[git status], status, '', "uncaught signal")

error = Git::SignaledError.new(result)

assert(error.is_a?(Git::GitExecuteError))
assert_equal(result, error.result)
end

def test_message
status = Struct.new(:to_s).new('pid 65628 SIGKILL (signal 9)') # `kill -9 $$`
result = Git::CommandLineResult.new(%w[git status], status, '', "uncaught signal")

error = Git::SignaledError.new(result)

expected_message = "[\"git\", \"status\"]\nstatus: pid 65628 SIGKILL (signal 9)\nstderr: \"uncaught signal\""
assert_equal(expected_message, error.message)
end
end
2 changes: 1 addition & 1 deletion tests/units/test_tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def test_tags

assert(r2.tags.detect{|t| t.name == 'third'}.objectish == r2.tags.detect{|t| t.name == 'fifth'}.objectish)

assert_raise Git::GitExecuteError do
assert_raise Git::FailedError do
r2.add_tag('third')
end

Expand Down
Loading