Skip to content

Commit 8992701

Browse files
jcouballjcouball
authored
Refactor error thrown when a git command fails (#622)
Signed-off-by: James Couball <jcouball@yahoo.com>
1 parent cf74b91 commit 8992701

17 files changed

+284
-17
lines changed

‎lib/git.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,21 @@
77
require 'git/base'
88
require 'git/branch'
99
require 'git/branches'
10+
require 'git/command_line_result'
1011
require 'git/config'
1112
require 'git/diff'
1213
require 'git/encoding_utils'
1314
require 'git/escaped_path'
15+
require 'git/failed_error'
16+
require 'git/git_execute_error'
1417
require 'git/index'
1518
require 'git/lib'
1619
require 'git/log'
1720
require 'git/object'
1821
require 'git/path'
1922
require 'git/remote'
2023
require 'git/repository'
24+
require 'git/signaled_error'
2125
require 'git/status'
2226
require 'git/stash'
2327
require 'git/stashes'

‎lib/git/command_line_result.rb

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# frozen_string_literal: true
2+
3+
module Git
4+
# The result of running a git command
5+
#
6+
# This object stores the Git command executed and its status, stdout, and stderr.
7+
#
8+
# @api public
9+
#
10+
class CommandLineResult
11+
# Create a CommandLineResult object
12+
#
13+
# @example
14+
# `true`
15+
# git_cmd = %w[git version]
16+
# status = $?
17+
# stdout = "git version 2.39.1\n"
18+
# stderr = ""
19+
# result = Git::CommandLineResult.new(git_cmd, status, stdout, stderr)
20+
#
21+
# @param git_cmd [Array<String>] the git command that was executed
22+
# @param status [Process::Status] the status of the process
23+
# @param stdout [String] the output of the process
24+
# @param stderr [String] the error output of the process
25+
#
26+
def initialize(git_cmd, status, stdout, stderr)
27+
@git_cmd = git_cmd
28+
@status = status
29+
@stdout = stdout
30+
@stderr = stderr
31+
end
32+
33+
# @attribute [r] git_cmd
34+
#
35+
# The git command that was executed
36+
#
37+
# @example
38+
# git_cmd = %w[git version]
39+
# result = Git::CommandLineResult.new(git_cmd, $?, "", "")
40+
# result.git_cmd #=> ["git", "version"]
41+
#
42+
# @return [Array<String>]
43+
#
44+
attr_reader :git_cmd
45+
46+
# @attribute [r] status
47+
#
48+
# The status of the process
49+
#
50+
# @example
51+
# `true`
52+
# status = $?
53+
# result = Git::CommandLineResult.new(status, "", "")
54+
# result.status #=> #<Process::Status: pid 87859 exit 0>
55+
#
56+
# @return [Process::Status]
57+
#
58+
attr_reader :status
59+
60+
# @attribute [r] stdout
61+
#
62+
# The output of the process
63+
#
64+
# @example
65+
# stdout = "git version 2.39.1\n"
66+
# result = Git::CommandLineResult.new($?, stdout, "")
67+
# result.stdout #=> "git version 2.39.1\n"
68+
#
69+
# @return [String]
70+
#
71+
attr_reader :stdout
72+
73+
# @attribute [r] stderr
74+
#
75+
# The error output of the process
76+
#
77+
# @example
78+
# stderr = "Tag not found\n"
79+
# result = Git::CommandLineResult.new($?, "", stderr)
80+
# result.stderr #=> "Tag not found\n"
81+
#
82+
# @return [String]
83+
#
84+
attr_reader :stderr
85+
end
86+
end

‎lib/git/failed_error.rb

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# frozen_string_literal: true
2+
3+
require 'git/git_execute_error'
4+
5+
module Git
6+
# This error is raised when a git command fails
7+
#
8+
# The git command executed, status, stdout, and stderr are available from this
9+
# object. The #message includes the git command, the status of the process, and
10+
# the stderr of the process.
11+
#
12+
# @api public
13+
#
14+
class FailedError < Git::GitExecuteError
15+
# Create a FailedError object
16+
#
17+
# @example
18+
# `exit 1` # set $? appropriately for this example
19+
# result = Git::CommandLineResult.new(%w[git status], $?, '', "failed")
20+
# error = Git::FailedError.new(result)
21+
# error.message #=>
22+
# "[\"git\", \"status\"]\nstatus: pid 89784 exit 1\nstderr: \"failed\""
23+
#
24+
# @param result [Git::CommandLineResult] the result of the git command including
25+
# the git command, status, stdout, and stderr
26+
#
27+
def initialize(result)
28+
super("#{result.git_cmd}\nstatus: #{result.status}\nstderr: #{result.stderr.inspect}")
29+
@result = result
30+
end
31+
32+
# @attribute [r] result
33+
#
34+
# The result of the git command including the git command and its status and output
35+
#
36+
# @example
37+
# `exit 1` # set $? appropriately for this example
38+
# result = Git::CommandLineResult.new(%w[git status], $?, '', "failed")
39+
# error = Git::FailedError.new(result)
40+
# error.result #=>
41+
# #<Git::CommandLineResult:0x00000001046bd488
42+
# @git_cmd=["git", "status"],
43+
# @status=#<Process::Status: pid 89784 exit 1>,
44+
# @stderr="failed",
45+
# @stdout="">
46+
#
47+
# @return [Git::CommandLineResult]
48+
#
49+
attr_reader :result
50+
end
51+
end

‎lib/git/git_execute_error.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# frozen_string_literal: true
2+
3+
module Git
4+
# This error is raised when a git command fails
5+
#
6+
class GitExecuteError < StandardError; end
7+
end

‎lib/git/lib.rb

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
1+
require 'git/failed_error'
12
require 'logger'
23
require 'tempfile'
34
require 'zlib'
45

56
module Git
6-
7-
class GitExecuteError < StandardError
8-
end
9-
107
class Lib
118

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

11291126
command_thread = nil;
11301127

1131-
exitstatus = nil
1128+
status = nil
11321129

11331130
with_custom_env_variables do
11341131
command_thread = Thread.new do
11351132
output = run_command(git_cmd, &block)
1136-
exitstatus = $?.exitstatus
1133+
status = $?
11371134
end
11381135
command_thread.join
11391136
end
11401137

11411138
@logger.info(git_cmd)
11421139
@logger.debug(output)
11431140

1144-
raise Git::GitExecuteError, "#{git_cmd}:#{output}" if
1145-
exitstatus > 1 || (exitstatus == 1 && output != '')
1141+
if status.exitstatus > 1 || (status.exitstatus == 1 && output != '')
1142+
result = Git::CommandLineResult.new(git_cmd, status, output, '')
1143+
raise Git::FailedError.new(result)
1144+
end
11461145

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

‎lib/git/signaled_error.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# frozen_string_literal: true
2+
3+
require 'git/git_execute_error'
4+
5+
module Git
6+
# This error is raised when a git command exits because of an uncaught signal
7+
#
8+
# The git command executed, status, stdout, and stderr are available from this
9+
# object. The #message includes the git command, the status of the process, and
10+
# the stderr of the process.
11+
#
12+
# @api public
13+
#
14+
class SignaledError < Git::GitExecuteError
15+
# Create a SignaledError object
16+
#
17+
# @example
18+
# `kill -9 $$` # set $? appropriately for this example
19+
# result = Git::CommandLineResult.new(%w[git status], $?, '', "killed")
20+
# error = Git::SignaledError.new(result)
21+
# error.message #=>
22+
# "[\"git\", \"status\"]\nstatus: pid 88811 SIGKILL (signal 9)\nstderr: \"killed\""
23+
#
24+
# @param result [Git::CommandLineResult] the result of the git command including the git command, status, stdout, and stderr
25+
#
26+
def initialize(result)
27+
super("#{result.git_cmd}\nstatus: #{result.status}\nstderr: #{result.stderr.inspect}")
28+
@result = result
29+
end
30+
31+
# @attribute [r] result
32+
#
33+
# The result of the git command including the git command, status, and output
34+
#
35+
# @example
36+
# `kill -9 $$` # set $? appropriately for this example
37+
# result = Git::CommandLineResult.new(%w[git status], $?, '', "killed")
38+
# error = Git::SignaledError.new(result)
39+
# error.result #=>
40+
# #<Git::CommandLineResult:0x000000010470f6e8
41+
# @git_cmd=["git", "status"],
42+
# @status=#<Process::Status: pid 88811 SIGKILL (signal 9)>,
43+
# @stderr="killed",
44+
# @stdout="">
45+
#
46+
# @return [Git::CommandLineResult]
47+
#
48+
attr_reader :result
49+
end
50+
end

‎tests/units/test_branch.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def test_branch_create_and_switch
7979
assert(git.status.untracked.assoc('test-file1'))
8080
assert(!git.status.added.assoc('test-file1'))
8181

82-
assert_raise Git::GitExecuteError do
82+
assert_raise Git::FailedError do
8383
git.branch('new_branch').delete
8484
end
8585
assert_equal(1, git.branches.select { |b| b.name == 'new_branch' }.size)

‎tests/units/test_command_line_result.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
require 'test_helper'
2+
3+
class TestCommamndLineResult < Test::Unit::TestCase
4+
def test_initialization
5+
git_cmd = Object.new
6+
status = Object.new
7+
stdout = Object.new
8+
stderr = Object.new
9+
10+
result = Git::CommandLineResult.new(git_cmd, status, stdout, stderr)
11+
12+
assert_equal(git_cmd, result.git_cmd)
13+
assert_equal(status, result.status)
14+
assert_equal(stdout, result.stdout)
15+
assert_equal(stderr, result.stderr)
16+
end
17+
end

‎tests/units/test_commit_with_empty_message.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ def setup
99
def test_without_allow_empty_message_option
1010
Dir.mktmpdir do |dir|
1111
git = Git.init(dir)
12-
assert_raises Git::GitExecuteError do
12+
assert_raises Git::FailedError do
1313
git.commit('', { allow_empty: true })
1414
end
1515
end

‎tests/units/test_failed_error.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
require 'test_helper'
2+
3+
class TestFailedError < Test::Unit::TestCase
4+
def test_initializer
5+
status = Struct.new(:to_s).new('pid 89784 exit 1')
6+
result = Git::CommandLineResult.new(%w[git status], status, '', "failed")
7+
8+
error = Git::FailedError.new(result)
9+
10+
assert(error.is_a?(Git::GitExecuteError))
11+
assert_equal(result, error.result)
12+
end
13+
14+
def test_message
15+
status = Struct.new(:to_s).new('pid 89784 exit 1')
16+
result = Git::CommandLineResult.new(%w[git status], status, '', "failed")
17+
18+
error = Git::FailedError.new(result)
19+
20+
expected_message = "[\"git\", \"status\"]\nstatus: pid 89784 exit 1\nstderr: \"failed\""
21+
assert_equal(expected_message, error.message)
22+
end
23+
end

‎tests/units/test_git_execute_error.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
require 'test_helper'
2+
3+
class TestGitExecuteError < Test::Unit::TestCase
4+
def test_is_a_standard_error
5+
assert(Git::GitExecuteError < StandardError)
6+
end
7+
end

‎tests/units/test_lib.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def test_commit_with_no_verify
6464
@lib.add('test_file_2')
6565

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

‎tests/units/test_log.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def test_get_log_path
7474
end
7575

7676
def test_log_file_noexist
77-
assert_raise Git::GitExecuteError do
77+
assert_raise Git::FailedError do
7878
@git.log.object('no-exist.txt').size
7979
end
8080
end

‎tests/units/test_remotes.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,10 @@ def test_fetch_command_injection
155155
origin = "--upload-pack=touch #{test_file};"
156156
begin
157157
git.fetch(origin, { ref: 'some/ref/head' })
158-
rescue Git::GitExecuteError
158+
rescue Git::FailedError
159159
# This is expected
160160
else
161-
raise 'Expected Git::GitExecuteError to be raised'
161+
raise 'Expected Git::Failed to be raised'
162162
end
163163

164164
vulnerability_exists = File.exist?(test_file)

‎tests/units/test_signaled_error.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
require 'test_helper'
2+
3+
class TestSignaledError < Test::Unit::TestCase
4+
def test_initializer
5+
status = Struct.new(:to_s).new('pid 65628 SIGKILL (signal 9)') # `kill -9 $$`
6+
result = Git::CommandLineResult.new(%w[git status], status, '', "uncaught signal")
7+
8+
error = Git::SignaledError.new(result)
9+
10+
assert(error.is_a?(Git::GitExecuteError))
11+
assert_equal(result, error.result)
12+
end
13+
14+
def test_message
15+
status = Struct.new(:to_s).new('pid 65628 SIGKILL (signal 9)') # `kill -9 $$`
16+
result = Git::CommandLineResult.new(%w[git status], status, '', "uncaught signal")
17+
18+
error = Git::SignaledError.new(result)
19+
20+
expected_message = "[\"git\", \"status\"]\nstatus: pid 65628 SIGKILL (signal 9)\nstderr: \"uncaught signal\""
21+
assert_equal(expected_message, error.message)
22+
end
23+
end

‎tests/units/test_tags.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def test_tags
4343

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

46-
assert_raise Git::GitExecuteError do
46+
assert_raise Git::FailedError do
4747
r2.add_tag('third')
4848
end
4949

0 commit comments

Comments
 (0)