Skip to content

Valet fetch-share-url issue fix#1285

Merged
driesvints merged 2 commits intolaravel:masterfrom
NasirNobin:valet-share-improvements
Oct 24, 2022
Merged

Valet fetch-share-url issue fix#1285
driesvints merged 2 commits intolaravel:masterfrom
NasirNobin:valet-share-improvements

Conversation

@NasirNobin
Copy link
Copy Markdown
Contributor

Every time I run valet share it opens the tunnel successfully, then shows an error message related to fetch-share-url with this error message.

cURL error 7: Failed to connect to 127.0.0.1 port 4041 after 0 ms: 
Connection refused (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)
for http://127.0.0.1:4041/api/tunnels
🔽 Video demonstration of the issue
CleanShot.2022-10-02.at.02.24.25.mp4

This error was happening for a while. I ignored this for a few months. A few days back I was collaborating with a co-worker and he thought valet share is broken as this error always popup for him as well. So I thought of finding the underlying issue.

The issue:

Tunnel URL checker Ngrok::currentTunnelUrl is supposed to retry 20 times until it finds a tunnel URL on 127.0.0.1:4040/api/tunnels endpoint. Then again it should retry with 127.0.0.1:4041 port.

But that loop was getting exited when Ngrok is returning a 200 response but the tunnel URL is still not in the response.

In another scenario, it wasn't reaching the 4041 port part as the retry helper is throwing an exception after trying 20 times on the 4040 port and the script is exiting before trying the 4041 port.

This PR should fix both issues.

@NasirNobin NasirNobin marked this pull request as ready for review October 2, 2022 08:54
Copy link
Copy Markdown
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@drbyte
Copy link
Copy Markdown
Contributor

drbyte commented Oct 19, 2022

Looks good to me. 👍

@driesvints driesvints merged commit 305736b into laravel:master Oct 24, 2022
@driesvints
Copy link
Copy Markdown
Member

Thanks @NasirNobin

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.

3 participants