Gitlab::Popen.popen should return a Process::Status object instead of a (possibly nil) status code
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
Sometimes, Gitlab::Popen.popen return nil cmd_status because we're getting cmd_status = wait_thr.value.exitstatus instead of returning a Process::Status object (i.e. cmd_status = wait_thr.value).
The problem is that:
stat.exitstatus → fixnum or nil Returns the least significant eight bits of the return code of stat. Only available if exited? is true.
By returning a Process::Status, it's up to the callers to properly check status.success? (or other checks, e.g. status.exited? etc.).
Currently, there are 46 calls to Gitlab::Popen.popen in CE, and 50 calls in EE.
The discussion that triggered this issue can be seen here: https://sentry.gitlap.com/gitlab/gitlabcom/issues/62097/activity/
/cc @stanhu @nick.thomas