Let the transport handles bad urls

[lmazuel]

See #11048

If a bad combination of parametrized host + incorrect client creation happen, do not crash while trying to build the Url, but build something, and let the transport decides what to do.

For requests, the new trace is now:

  File "c:\users\lmazuel\git\azure-sdk-for-python\sdk\core\azure-core\azure\core\pipeline\transport\_requests_basic.py", line 284, in send
    raise error
azure.core.exceptions.ServiceRequestError: Invalid URL 'text/analytics/v3.0/krpratic-textanalytics.cognitiveservices.azure.com//text/analytics/v3.0/sentiment?showStats=false': No schema supplied. Perhaps you meant http://text/analytics/v3.0/krpratic-textanalytics.cognitiveservices.azure.com//text/analytics/v3.0/sentiment?showStats=false?

For aiohttp:

aiohttp.client_exceptions.InvalidURL: text/analytics/v3.0/krpratic-textanalytics.cognitiveservices.azure.com//text/analytics/v3.0/sentiment?showStats=false

which is way better than the current error you can see on issue #11048

File "C:\Users\krpratic\azure-sdk-for-python\env\lib\site-packages\azure\core\pipeline\transport\_base.py", line 695, in format_url
    base = self._base_url.format(**kwargs).rstrip("/")
KeyError: 'Endpoint'

EDIT: Will now raise an exception

"The value provided for the url part Endpoint was incorrect, and resulted in an invalid url"

[kristapratico]

Thank you so much for doing this!

[xiangyan99]

If the input kwargs do not satisfy template, why not raise error?

I have some concerns if we modify the template silently.

Maybe we should just give a more descriptive error message?

[lmazuel]

What error would you raise? You don't have enough information other that "something is wrong", building the url leniently and let the transport decide if it's malformed sounds fair to me. What are your concerns?

[xiangyan99]

My concern is the case that after we skip some components, it is still a valid URL but returns different response. e.g. api_version_number. Because we skip it silently for user, it may confuse users.

[lmazuel]

@xiangyan99 if I catch the error, only thing I know is that something the customer provided is not right. I don't know if it's a client parameter (like endpoint), I don't know if it's an operation parameter, I don't know if it's both.

I could raise something like this:

Unable to build a valid Url, one of your client parameter or operation parameter is invalid, and doesn't allow to build a correct url.

Thoughts?

[annatisch]

We can likely include the contents of kwargs in the error - as I think these values will mostly be user-supplied, with some values added by the generator, but still potentially useful in debugging.

[xiangyan99]

I think the point is if there is something wrong, we would like it to fail on compile time (when building the url) than un-deterministic result (may or may not fail when sending it to server). Your ideas?

[lmazuel]

kwargs is empty, the only thing we know is that Endpoint (the Swagger template part) was invalid. I don't have the Python name related to that Swagger template part (it's likely the lower case in snake case, like FooBar template part would be foo_bar parameter name, but I don't have the actual mapping)

is this better:

The value provided for the url part Endpoint was incorrect, and resulted in an invalid url

Thoughts?

Edit: Endpoint I mentioned could be anything, the Swagger decides, could be UrlEndpoint, UrlBase, etc. Endpoint is the example for Text Analytics here

[kristapratico]

I've seen users run into this error in two ways:

1. accidentally chop off https
2. define the endpoint ending with a comma (making it a tuple) e.g.:
   endpoint = "https://python-textanalytics.cognitiveservices.azure.com/",

In the first case, we do get a good error back from the transport. In the second case, we get this:

azure.core.exceptions.ServiceRequestError: No connection adapters were found for 'text/analytics/v3.0/('https://python-textanalytics.cognitiveservices.azure.com/',)/text/analytics/v3.0/sentiment?showStats=false'

... which is not so good.

So in that case I would be happy with this:

The value provided for the url part Endpoint was incorrect, and resulted in an invalid url

[rakshith91]

accidentally chop off https is often handles individually by clients - we do it in storage, search etc.
I think this would end up with a KeyError somewhere.

[lmazuel]

@annatisch @rakshith91 @kristapratico @xiangyan99 I pushed an update, how does it sound?

[kristapratico]

I'm good with this change