Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI improvement, error messages show OCaml exceptions/objects #5954

Open
patrick-nicodemus opened this issue May 15, 2024 · 1 comment
Open
Labels

Comments

@patrick-nicodemus
Copy link

This is just an appearance thing, so it's relatively minor but I think worth posting in case anyone else thinks it's worth changing.

On a failed download attempt I get the following error messages:

<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] Could not update repository "coq-released":
        OpamDownload.Download_fail(_, "Curl failed: \"/usr/bin/curl --write-out
        %{http_code}\\\\n --retry 3 --retry-delay 2 --user-agent opam/2.1.5 -L
        -o /tmp/opam-4594-455670/index.tar.gz.part --
        https://coq.inria.fr/opam/released/index.tar.gz\" exited with code 6")
[ERROR] Could not update repository "default": OpamDownload.Download_fail(_,
        "Curl failed: \"/usr/bin/curl --write-out %{http_code}\\\\n --retry 3
        --retry-delay 2 --user-agent opam/2.1.5 -L -o
        /tmp/opam-4594-679cce/index.tar.gz.part --
        https://opam.ocaml.org/index.tar.gz\" exited with code 6")

The messages are not hard to read and understand but it seems like the raw exception itself being leaked to the end user is not clean. I think this should be pretty-printed somehow. It also doesn't seem great that %{http-code} is in the error message instead of the actual http code that was used. Is this a necessary difficulty with dealing with format strings in error handling or can the actual http code be shown to the user?

@leviroth
Copy link

leviroth commented Oct 28, 2024

#6107 looks like it provides a much more readable error-reporting scheme. (As I note in my comment there, the PR in its present form actually makes this specific example worse, but presumably that's fixable and then the resulting error is a lot nicer.)

I think this would be pretty helpful in helping newcomers troubleshoot issues, or with helping others help newcomers. I actually found this post after someone on Discord asked about the following, which was just a little tedious to turn into a debuggable command:

OpamDownload.Download_fail(_, "curl failed: \"C:\\\\windows\\\\system32\\\\curl.exe --write-out %{http_code}\\\\n --retry 3 --retry-delay 2 --user-agent opam/2.2.1 -L -o C:\\\\Users\\\\theo2\\\\AppData\\\\Lo\
cal\\\\Temp\\\\opam-7180-769b82\\\\sha512.sum.part -- https://cygwin.com/sha512.sum\" exited with code 35")  

(And, ideally the actual error output of curl would not be lost so that we wouldn't be required to recover the original curl command anyway; I might take a stab at that.)

It also doesn't seem great that %{http-code} is in the error message instead of the actual http code that was used. Is this a necessary difficulty with dealing with format strings in error handling or can the actual http code be shown to the user?

The string %{http-code} - literally those 12 bytes - is what's passed to curl, see https://curl.se/docs/manpage.html#httpcode. So, I think it makes sense to preserve it in the error message. It might make sense to present the HTTP status code where there is one, though in the particular case of curl error 6 I assume that there is none.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants