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.

Sign up for GitHub

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

Jump to bottom

Improve error message when receiving unexpected server responses #5341

Open
hojberg opened this issue Sep 11, 2024 · 3 comments · May be fixed by #5342
Open

Improve error message when receiving unexpected server responses #5341

hojberg opened this issue Sep 11, 2024 · 3 comments · May be fixed by #5342
Labels
error-message Request for improved error message

Comments

@hojberg
Copy link
Member

hojberg commented Sep 11, 2024 •
edited
Loading

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
@hojberg hojberg added the error-message Request for improved error message label Sep 11, 2024
This was referenced Sep 11, 2024
Open
Open
@ceedubs
Copy link
Contributor

ceedubs commented Sep 11, 2024

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
Copy link
Member Author

hojberg commented Sep 11, 2024 •
edited
Loading

@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
Copy link
Member Author

hojberg commented Sep 11, 2024 •
edited
Loading

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

@hojberg hojberg linked a pull request Sep 11, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
error-message Request for improved error message
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Summarize unexpected server responses unisonweb/unison
2 participants
@hojberg @ceedubs