Skip to content

Commit 2704e65

Browse files
CopilotpaveraCopilot
authored
Remove enable_shared_helpers_command_timeout feature flag (#14125)
* Remove enable_shared_helpers_command_timeout feature flag from shared_helpers_spec.rb Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Remove enable_shared_helpers_command_timeout experiment mocks from all spec files Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Fix Sorbet type errors by defaulting stdout/stderr to empty strings Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Fix Sorbet type errors: use T.let for stdout/stderr type narrowing Replace `||=` pattern with explicit T.let to properly narrow T.nilable(String) to String, satisfying Sorbet strict type checking. Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Fix CI failures: update Open3 mocks to SharedHelpers and handle EPIPE - github_actions/update_checker_spec.rb: Replace Open3.capture2e mocks with Dependabot::SharedHelpers.run_shell_command mocks since the feature flag removal means run_shell_command now always uses CommandHelpers.capture3_with_timeout (which uses Open3.popen3 internally) - command_helpers.rb: Catch Errno::EPIPE during stdin.write in capture3_with_timeout - this occurs when the process exits before reading its input (e.g. the `false` builtin used in npm tests) Co-authored-by: pavera <660677+pavera@users.noreply.github.com> * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove process startup error handling Removed error handling for process startup failure. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pavera <660677+pavera@users.noreply.github.com> Co-authored-by: Tom Christensen <pavera@github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 57ab757 commit 2704e65

26 files changed

Lines changed: 132 additions & 383 deletions

File tree

bun/spec/dependabot/bun/file_updater_spec.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@
5757

5858
before do
5959
FileUtils.mkdir_p(tmp_path)
60-
allow(Dependabot::Experiments).to receive(:enabled?)
61-
.with(:enable_shared_helpers_command_timeout).and_return(true)
6260
allow(Dependabot::Experiments).to receive(:enabled?)
6361
.with(:avoid_duplicate_updates_package_json).and_return(false)
6462
end

bun/spec/dependabot/bun/update_checker/version_resolver_spec.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@
7575
.to_return(status: 200, body: opentelemetry_api_registry_response)
7676
stub_request(:get, opentelemetry_context_async_hooks_registry_listing_url)
7777
.to_return(status: 200, body: opentelemetry_context_async_hooks_registry_response)
78-
allow(Dependabot::Experiments).to receive(:enabled?)
79-
.with(:enable_shared_helpers_command_timeout).and_return(true)
8078
end
8179

8280
after do

bun/spec/dependabot/bun/update_checker_spec.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@
6363
.to_return(status: 200, body: registry_response)
6464
stub_request(:head, "#{registry_base}/#{dependency_name}/-/#{unscoped_dependency_name}-#{target_version}.tgz")
6565
.to_return(status: 200)
66-
allow(Dependabot::Experiments).to receive(:enabled?)
67-
.with(:enable_shared_helpers_command_timeout).and_return(true)
6866
end
6967

7068
after do

common/lib/dependabot/command_helpers.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,11 @@ def self.capture3_with_timeout(
103103
Dependabot.logger.info("Started process PID: #{pid} with command: #{sanitized_env_cmd.join(' ')}")
104104

105105
# Write to stdin if input data is provided
106-
stdin&.write(stdin_data) if stdin_data
106+
begin
107+
stdin&.write(stdin_data) if stdin_data
108+
rescue Errno::EPIPE
109+
# Process exited before reading stdin - continue to collect output
110+
end
107111
stdin&.close
108112

109113
stdout_io.sync = true

common/lib/dependabot/shared_helpers.rb

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,13 @@ def self.run_helper_subprocess(
163163
end
164164

165165
env_cmd = [env, cmd].compact
166-
if Experiments.enabled?(:enable_shared_helpers_command_timeout)
167-
stdout, stderr, process = CommandHelpers.capture3_with_timeout(
168-
env_cmd,
169-
stdin_data: stdin_data,
170-
timeout: timeout
171-
)
172-
else
173-
stdout, stderr, process = T.unsafe(Open3).capture3(*env_cmd, stdin_data: stdin_data)
174-
end
166+
raw_stdout, raw_stderr, process = CommandHelpers.capture3_with_timeout(
167+
env_cmd,
168+
stdin_data: stdin_data,
169+
timeout: timeout
170+
)
171+
stdout = T.let(raw_stdout || "", String)
172+
stderr = T.let(raw_stderr || "", String)
175173
time_taken = Time.now - start
176174

177175
if ENV["DEBUG_HELPERS"] == "true"
@@ -480,22 +478,16 @@ def self.run_shell_command(
480478
opts[:chdir] = cwd if cwd
481479

482480
env_cmd = [env || {}, cmd, opts].compact
483-
if Experiments.enabled?(:enable_shared_helpers_command_timeout)
484-
kwargs = {
485-
stderr_to_stdout: stderr_to_stdout,
486-
timeout: timeout
487-
}
488-
kwargs[:output_observer] = output_observer if output_observer
489-
490-
stdout, stderr, process = CommandHelpers.capture3_with_timeout(
491-
env_cmd,
492-
**kwargs
493-
)
494-
elsif stderr_to_stdout
495-
stdout, process = Open3.capture2e(env || {}, cmd, opts)
496-
else
497-
stdout, stderr, process = Open3.capture3(env || {}, cmd, opts)
498-
end
481+
kwargs = {
482+
stderr_to_stdout: stderr_to_stdout,
483+
timeout: timeout
484+
}
485+
kwargs[:output_observer] = output_observer if output_observer
486+
487+
stdout, stderr, process = CommandHelpers.capture3_with_timeout(
488+
env_cmd,
489+
**kwargs
490+
)
499491

500492
time_taken = Time.now - start
501493

0 commit comments

Comments
 (0)