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

refactor: show more info about connection errors in output #4621

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

mitchellwrosen
Copy link
Member

Overview

Addresses #4619; I thought it might be find to just print out the whole error for the user, rather than hide it behind a friendlier but less informative "something went wrong".

However, I now see in the bug report that "we don't want to dump out cryptic errors normally". So, this might need more work. Nonetheless I thought I'd throw it up for consideration.

@ceedubs
Copy link
Contributor

ceedubs commented Jan 16, 2024

Thanks @mitchellwrosen! I know that it's different than what @aryairani said, but my personal opinion is that for a tool that targets developers a verbose error message is better than failure without any indication of what's going wrong.

Minor nitpick: I don't know what all errors can be represented by these two constructors, but I think that some (if not all) of them happen when you can't actually connect to the server so saying that you got an error from the server might not quite be accurate.

@aryairani
Copy link
Contributor

Thanks @mitchellwrosen; I will get a second opinion about the cryptic errors.
@ceedubs Can you use this draft PR build to answer your question in the mean time? Or no, you need it to be in a release to get it into your sandbox?

I guess we don't know exactly how to trigger the errors to even know how cryptic they will be, right?

@ceedubs
Copy link
Contributor

ceedubs commented Jan 17, 2024

I'm attempting to try from this branch and am waiting on a lengthy build. I'll leave an update when it finishes.

@ceedubs
Copy link
Contributor

ceedubs commented Jan 17, 2024

Worked great! The problem was one on my end and resulted in the following error message:

   tmp/main> pull @unison/httpserver/releases/3.0.2 lib.httpserver_3_0_2

      Oops, I encountered an unexpected exception from the server.

        InternalException
            ( HandshakeFailed
                ( Error_Protocol
                    ( "certificate has unknown CA"
                    , True
                    , UnknownCa
                    )
                )
            )

@mitchellwrosen
Copy link
Member Author

What should we do with this one @aryairani?

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

Successfully merging this pull request may close these issues.

None yet

3 participants