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

Improve error messages for failed connections #270

Merged
merged 6 commits into from
Sep 3, 2021

Conversation

clue
Copy link
Member

@clue clue commented Sep 3, 2021

This changeset improves error messages for failed connections. In particular, failed TCP/IP connections using the happy eyeballs connector (the default) or the old DNS connector will now report the hostname once at the beginning of the message and will not show its internal hostname parameter multiple times:

- Connection to tcp://localhost:80 failed: Last error for IPv4: Connection to tcp://127.0.0.1:80?hostname=localhost failed: Connection refused. Previous error for IPv6: Connection to tcp://[::1]:80?hostname=localhost failed: Connection refused
+ Connection to tcp://localhost:80 failed: Last error for IPv4: Connection to tcp://127.0.0.1:80 failed: Connection refused. Previous error for IPv6: Connection to tcp://[::1]:80 failed: Connection refused

Likewise, we now make sure to always consistently include the URI scheme used in the error message. For instance, failed TLS connections will now include the appropriate URI scheme for the start of the message and show the underlying URI scheme for underlying TCP/IP connection issues such as this:

- Connection to localhost:443 failed: Last error for IPv4: Connection to tcp://127.0.0.1:443?hostname=localhost failed: Connection refused. Previous error for IPv6: Connection to tcp://[::1]:443?hostname=localhost failed: Connection refused
+ Connection to tls://localhost:443 failed: Last error for IPv4: Connection to tcp://127.0.0.1:443 failed: Connection refused. Previous error for IPv6: Connection to tcp://[::1]:443 failed: Connection refused

Builds on top of #265, #266, #267 and others such as clue/reactphp-redis#116, friends-of-reactphp/mysql#141 and more

@clue clue added this to the v1.10.0 milestone Sep 3, 2021
@clue
Copy link
Member Author

clue commented Sep 3, 2021

Updated to fix the build error on PHP < 7.4 by cleaning up any garbage memory references as also done in reactphp/dns#118 and reactphp/dns#160.

This is now ready for review :shipit:

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

Successfully merging this pull request may close these issues.

None yet

2 participants