github: produce better curl error messages.

[MikeMcQuaid]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew tests with your changes locally?

---

If we don't know why curl has failed then ensure that the error messages that it produced are included as part of the user output.

Prompted by Homebrew/homebrew-core#2410. CC @ilovezfs

[UniqMartin]

Open3.popen3in Ruby 1.8.7 unfortunately doesn't provide the fourth status block argument, thus things will fail further up the call chain.

[MikeMcQuaid]

Suggestions for an API to use instead for Ruby 1.8.7?

[UniqMartin]

Unfortunately, I'm not aware of a somewhat elegant solution …

We could re-implement Open3.popen3 adapting it from a more modern Ruby, as I believe all the building blocks exist, but this surely feels like a bad solution.

[ilovezfs]

how much code is it

[UniqMartin]

At most a few dozen. Mostly things like setting up pipes, then fork and exec with some massaging of the pipes depending on whether a block was given or not.

[MikeMcQuaid]

Yuck. Given the imminent Ruby 2 usage I might just figure out an interim solution.

[UniqMartin]

I'd probably lean towards using the new code for Ruby 2.0+ and falling back to Utils.popen_read_text for older Rubies. That way most of our users will benefit from an improved error handling and we can rip out the legacy code path as soon as we enforce Ruby 2.0+. A very rough and untested sketch (that would also require some minor adjustments on the call site):

def curl_output(*args)
  curl_args = curl_args(args) - ["--fail"]
  if RUBY_VERSION.split(".").first.to_i >= 2
    result = Open3.popen3(*curl_args) do |_, stdout, stderr, status|
      [stdout.read, stderr.read, status]
    end
    result[0..1] + [result.last.success?]
  else
    output = Utils.popen_read_text(*curl_args)
    [output, "(Curl error message unavailable.)", $?.success?]
  end
end

[MikeMcQuaid]

Done!

[ilovezfs]

@MikeMcQuaid Since the problem in Homebrew/homebrew-core#2410 seems not to go away when the firewall is up, I wonder if there's a solution to recommend so that user isn't always looking at these error messages. For instance, do we know what port the GitHub API requires so the appropriate exception can be added to the firewall rules if desired?

[ilovezfs]

We could also recommend setting HOMEBREW_NO_GITHUB_API=1 but that feels like a sledgehammer solution since the problem may only be transient (e.g., you're using a restaurant's WiFi, which has a firewall on the relevant port, but no such problem at home) but setting a profile variable is unlikely to be unwound spontaneously by the user at some later date.

[UniqMartin]

The GitHub API uses HTTPS, so I guess we're talking about port 443. I don't think there's much we can do if this port happens to be blocked, but in this case the user typically will have quite a problematic web experience in general …

[MikeMcQuaid]

@ilovezfs yeh, if they cannot access the GitHub API they are going to have to just disable all usage.

[ilovezfs]

Should the output say that explicitly as well as possible cause or just wait for them to file the issues and respond each time?

[MikeMcQuaid]

@ilovezfs Sorry, you've lost me: what output/what cause?

[ilovezfs]

@MikeMcQuaid Oh, I'm only asking if you think the curl error messages will need any brew customization to make them more clear or if they're good enough all by themselves.

[MikeMcQuaid]

@ilovezfs Good point. I think the curl error messages would be an improvement on what we have now but may need further tweaks eventually, yeh.

[UniqMartin]

👍

[UniqMartin]

Other than the bug I found (and you already acknowledged) it behaved as I expected in local testing (e.g. when switching off WiFi or using Ruby 1.8.7).

👍 as soon as the bug is squashed.