`GitHelper.checkout_and_pull` swallows git errors and reports failure only via its return value

[jkmassel]

Fastlane::Helper::GitHelper.checkout_and_pull swallows all git errors and reports failure only through its return value, so the easy way to call it — ignoring the return — is also the unsafe way.

Summary

• checkout_and_pull rescues StandardError and returns true/false instead of raising.
• A caller that ignores the return (the natural, and by far the most common, usage across our repos) silently proceeds after a failed git checkout or git pull.
• The failure behavior is inconsistent with its sibling helpers and can't be inferred from the method name (no !), so callers don't realize they need to guard it.

def self.checkout_and_pull(branch)
  branch = branch.first.join('/') if branch.is_a?(Hash)
  Action.sh('git', 'checkout', branch)
  Action.sh('git', 'pull')
  true
rescue StandardError
  false
end

The inconsistency

GitHelper has no consistent convention for how git failures surface, and the Ruby ! convention is arguably inverted here — the raising helper carries a bang, the silently-swallowing one doesn't:

Method	On git failure	Bang?
checkout_and_pull	swallows → returns false	no
create_branch	raises (no rescue)	no
commit	swallows → returns false	no
delete_local_branch_if_exists!	raises (ruby-git)	yes

How this would have helped us

In WordPress-Android#23001 we added a scheduled lane that resets a rolling branch to trunk, re-downloads translations, and opens a single rolling PR. Its first line is:

Fastlane::Helper::GitHelper.checkout_and_pull(DEFAULT_BRANCH)

The job runs on a persistent mac-metal agent with a reused checkout. If the git pull here fails — a network blip, or a stale/diverged local trunk left by a prior run — the error is swallowed and the lane proceeds to cut the sync branch from a stale trunk, then force-pushes and opens/refreshes a PR against the wrong base. Nothing aborts; the failure is invisible until someone notices the PR is behind.

Because the toolkit offers no raising variant, we had to re-implement the guard at the call site:

unless Fastlane::Helper::GitHelper.checkout_and_pull(DEFAULT_BRANCH)
  UI.user_error!("Could not check out and pull #{DEFAULT_BRANCH}; aborting translation sync.")
end

Every caller that wants the safe behavior has to write this same boilerplate. A raising default (or an obvious raising variant) would have made the safe path the default one.

Proposed fix

1. Add a raising variant — checkout_and_pull! that raises on failure, keeping the bool version for the (rarer) callers that genuinely want to branch on the result. This matches Ruby's ! convention and is non-breaking.
2. Or raise by default with an opt-out — checkout_and_pull(branch, fatal: true), defaulting fatal to true so the safe behavior is the default. (Breaking for any caller currently relying on the false return.)
3. At minimum, document that the return value must be checked, and align the !-naming across GitHelper so failure behavior is inferable from the signature.

What we're not asking for

This isn't a blanket "make every helper raise." commit returning a bool is genuinely useful — "nothing to commit" is a common, valid no-op that callers want to sail past (the same WordPress-Android lane relies on exactly that for its prune step). The point is narrower: a failed checkout_and_pull is almost never something a caller wants to continue past, so for this helper the safe path should be the default rather than opt-in boilerplate.