alexdlaird / pyngrok

A Python wrapper for ngrok
https://pyngrok.readthedocs.io
MIT License
416 stars 59 forks source link

Cannot `connect` with `labels` #116

Closed mapacheverdugo closed 9 months ago

mapacheverdugo commented 10 months ago

Describe the Bug

Now ngrok team made static domains available for all users so I wanna test it with pyngrok but this error is returned:

File ".../pyngrok/ngrok.py", line 488, in api_request
    raise PyngrokNgrokHTTPError("ngrok client exception, API returned {}: {}".format(status_code, response_data),
pyngrok.exception.PyngrokNgrokHTTPError: ngrok client exception, API returned 400: {"error_code":102,"status_code":400,"msg":"invalid tunnel configuration","details":{"err":"yaml: unmarshal errors:\n  line 1: field proto not found in type config.LabeledTunnel"}}

Steps to Reproduce

http_tunnel = ngrok.connect(
    addr="http://localhost:3000",
    labels=["edge=edghts_example"]
)

Environment

mapacheverdugo commented 10 months ago

I think this is related to #110 and maybe #113. This error can be fixed by removing the use of the default proto value when the tunnel is labeled

alexdlaird commented 10 months ago

I believe you're correct, this seems related to #113, which is to say, it's known Edge and its features aren't supported by pyngrok (because they aren't available via the API client to ngrok, which is how pyngrok interactions with the binary). I'll have to take a look at your suggestion to remove the proto default though, perhaps this is a way we could add support for this in the future.

mapacheverdugo commented 10 months ago

I've been trying a few things, and it turns out that the following if-block solves the problem:

if "labels" in options:
        options.pop("proto")
        options.pop("schemes")

api_url = get_ngrok_process(pyngrok_config).api_url

The ngrok API returned the tunnel URL as an empty string, which is the expected behavior of the API for this case.

IMO this little fix becomes necessary due to the change the ngrok team made for free users, even if it doesn't mean full Edge support, so please consider adding it 🙏🏻

(I tried to make a PR, but I couldn't successfully pass all the tests after clone the project, so I don't know if I'm missing something 😭 )

alexdlaird commented 10 months ago

If you could, throw what you do have currently in to a PR, with a test that validates your use case. If other tests are broken, I'll help fix them to get the PR passed and merged, since with the frequency lately, it does sound like this is something that's changed for free accounts that we should add support for.

alexdlaird commented 10 months ago

I actually took a stab at this myself, since it's come up so much recently. I will most likely change the API for this, but I have a WIP PR, the branch is support-labels, if you want to have a look at the test and try it out for yourself.

https://github.com/alexdlaird/pyngrok/pull/117

alexdlaird commented 10 months ago

When the PR is merged in the next day or two, you'll see basic support for ngrok's Cloud Edge in pyngrok 6.1.0.