unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.81k stars 271 forks source link

Improve error message when receiving unexpected server responses #5341

Open hojberg opened 2 months ago

hojberg commented 2 months ago

Note, that while this example shows a specific command that could have a dedicated error message, this suggested ticket is about improving the unexpected server response message generally.

What's the message you're seeing? Please paste from your terminal or paste a screenshot, e.g:

focus/main> lib.install potions

  Oops, I received an unexpected status code from the server.

  Here is the request.

    Request
        { requestPath =
            ( BaseUrl
                { baseUrlScheme = Https
                , baseUrlHost = "api.unison-lang.org"
                , baseUrlPort = 443
                , baseUrlPath = ""
                }
            , "/ucm/v1/projects/project"
            )
        , requestQueryString = fromList
            [
                ( "name"
                , Just "potions"
                )
            ]
        , requestBody = Nothing
        , requestAccept = fromList
            [ application/json;charset=utf-8
            , application/json
            ]
        , requestHeaders = fromList []
        , requestHttpVersion = HTTP/1.1
        , requestMethod = "GET"
        }

  Here is the full response.

    Response
        { responseStatusCode = Status
            { statusCode = 400
            , statusMessage = "Invalid Parameter"
            }
        , responseHeaders = fromList
            [
                ( "Server"
                , "nginx/1.24.0"
                )
            ,
                ( "Date"

What would a better version look like?

Sorry, I wasn't able to perform `lib.install`:

  The server (share.unison-lang.org) responded 
  unexpectedly with: 
    400, Invalid Parameter
ceedubs commented 2 months ago

The error message in the report is truncated. Crucially, it includes this line:

, responseBody = "Unable to parse parameter name, Project shorthand must be of the form @user/project"

I think that ucm should definitely handle this case better (so it doesn't lead to an HTTP error). But I think that I disagree that removing the details of the request results is improving the error message. This is a tool used solely by software developers; they are probably familiar with HTTP and the details might help them provide a better error message for a bug report (if not address the issue themself).

hojberg commented 2 months ago

@ceedubs I'm happy to include the response body in the error message. My general goal here is to not dump Haskell data structures. While our users are developers they are not all Haskell developers and they are also not familiar with the intricacies of our systems (and shouldn't need to be to successfully write unison code).

In this case I think we should also parse the project name before even sending the request and give an appropriate contextual error.

hojberg commented 2 months ago

Suggestion including the response body:

Sorry, I wasn't able to perform `lib.install`. Unison Share responded unexpectedly with: 

  Status 400, Invalid Parameter
  Unable to parse parameter name, Project shorthand must be of the form @user/project