diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f007c..9f71369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,17 +9,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +- `InferenceClient`, `HTTPClient` and the authentication token requests no longer follow HTTP redirects. A redirect response now raises `InferenceClientError` or `APIException`. A redirect is a 301, 302, 303, 307 or 308 response with a `Location` header. The error message includes the `Location` value. Other 3xx responses, such as 304, go back to the caller as before. - `InferenceClient` now checks the request `path` with the same rules as the main API client. A path built from untrusted input could inject query parameters or a fragment into a request that carries the inference key, and dot-segment forms such as `..;/` and `..%252f` passed the old check. `path` may now contain only letters, digits, `-._~`, `/` and `:`, and must not contain a `.` or `..` segment. Anything else raises `InferenceClientError` before the request is sent. ### Added +- `InferenceClient` has a new `follow_redirects` argument. It is off by default. When it is on, the client follows a redirect only if the target is inside the deployment. The target must have the same scheme, host and port, and a path under `endpoint_base_url` that passes the same rules as `path`. The client follows at most 5 redirects. Any other redirect raises `InferenceClientError`. `AsyncInferenceExecution.status_json()` and `result()` never follow a redirect. +- When a redirect only adds or removes a trailing slash, the `InferenceClientError` message tells you to change `path`. - `InferenceClient.run_sync`, `InferenceClient.run`, `Deployment.run_sync` and `Deployment.run` accept a `params` argument for query string parameters, like the other `InferenceClient` methods. ### Changed +- `AsyncInferenceExecution.status_json()` and `result()` now use the client `timeout_seconds` and raise `InferenceClientError` on an HTTP error status. +- The `InferenceClient` methods `get`, `post`, `put`, `delete`, `patch`, `head`, `options` and `health` now use the client `timeout_seconds` when you do not give `timeout_seconds`. Before, these calls had no timeout. To wait longer, set `timeout_seconds` on the client or on the call. +- `HTTPClient` methods ignore the `allow_redirects` keyword argument. +- An `InferenceClientError` for an HTTP error status now ends with the response body, cut to 500 characters. For a failed async job, `result()` returns the deployment's error status, so the error now shows the deployment's message. - **Breaking:** `InferenceClient` no longer accepts a query string or fragment in `path`. Pass query data as `params` instead. For example, replace `client.get('v1/models?limit=10')` with `client.get('v1/models', params={'limit': 10})`, and `client.run_sync(data, path='v1/completions?stream=true')` with `client.run_sync(data, path='v1/completions', params={'stream': 'true'})`. - **Breaking:** `InferenceClient` now accepts only letters, digits, `-._~`, `/` and `:` in `path`. A path with a percent-encoded value (such as `%20` or `%2F`), a space, a non-ASCII character or another character such as `@`, `+`, `,`, `=` or `;` raises `InferenceClientError`. Send such values in the request body or in `params`. +### Fixed + +- `AsyncInferenceExecution.status_json()` no longer raises `ValueError` for a failed job. `AsyncStatus` has a new value, `Error`. The status JSON has an `Error` field with the reason. +- `AsyncInferenceExecution.result()` now parses a JSON result when the `Content-Type` has parameters, for example `application/json; charset=utf-8`. A result without a `Content-Type` header no longer raises `KeyError`. + ## [1.25.0] - 2026-10-23 ### Deprecated diff --git a/CLAUDE.md b/CLAUDE.md index a5b31b4..387c1d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -105,11 +105,15 @@ Reject, do not encode: `requests` decodes `%2E` back to `.` before sending, and All five verbs delegate to a single private `_request`, which is where the url is built and validated. Add new verbs by delegating to it, never by calling `requests` directly. +**Never follow a redirect.** Every request that carries credentials is sent with `allow_redirects=False`, and a redirect raises: `handle_error` for `HTTPClient` and the auth token calls, and `InferenceClient._send` for inference. `requests` keeps `Authorization` on a same-host redirect, and a 307/308 also keeps the method and body. The `Location` target never went through path validation. Any new send site must pass `allow_redirects=False` and go through the existing error check. Inference requests, including `AsyncInferenceExecution` status and result, go through `InferenceClient._send`, never `_session` directly. A redirect is `requests.Response.is_redirect`: 301, 302, 303, 307 or 308 with a `Location` header, the same rule `requests` uses to follow. Any other 3xx, such as 304, goes back to the caller. + +The one exception is `InferenceClient(follow_redirects=True)`, an opt-in for deployment calls only. `_send` then passes `allow_redirects=True`, and `_RedirectPolicySession.get_redirect_target` reports a target only if `_is_inside_deployment` accepts it: same scheme, host and port as `endpoint_base_url`, a path equal to the deployment path or under `/`, a raw `Location` path that passes `has_relative_path_segment`, and a target path under the deployment path that passes `check_request_path` with `_ALLOWED_IN_INFERENCE_PATH`, the same check as `path`. A rejected target stops `resolve_redirects`, and `_send` raises on the remaining redirect. The session caps hops at 5. `AsyncInferenceExecution` status and result never follow, because those urls belong to the gateway. Do not add the flag to `HTTPClient` or the auth token calls. + The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `path_params` is the only supported way to put a caller-supplied value into a path. `_add_base_url` re-asserts the same allowlist on the finished path, as a backstop for a call site that skips `path_params`. The finished-path check is `verda.helpers.check_request_path`. It rejects `?`, `#`, any character outside an allowlist, and `.`/`..` segments. `HTTPClient._add_base_url` calls it with the default allowlist (unreserved set plus `/`). `InferenceClient._build_url` calls it on the caller's multi-segment `path` with `_ALLOWED_IN_INFERENCE_PATH`, which also allows `:` (TensorFlow Serving's `my_model:predict`). Query data for both clients goes in `params`, never in the path. Do not re-implement the check. -`verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for `InferenceClient`'s `endpoint_base_url` only. Do not use it to check a request path. +`verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for two values only: `InferenceClient`'s `endpoint_base_url`, and the raw `Location` path in the `follow_redirects` check (`_is_inside_deployment`), where it runs together with `check_request_path`. Do not use it to check a request path that the SDK builds. ## Code style diff --git a/docs/source/examples/containers/index.rst b/docs/source/examples/containers/index.rst index 324ce2f..8bd7359 100644 --- a/docs/source/examples/containers/index.rst +++ b/docs/source/examples/containers/index.rst @@ -17,4 +17,5 @@ This section contains examples demonstrating how to work with containers in Verd scaling inference_async inference_sync - inference_minimal \ No newline at end of file + inference_minimal + inference_redirects diff --git a/docs/source/examples/containers/inference_redirects.rst b/docs/source/examples/containers/inference_redirects.rst new file mode 100644 index 0000000..3507741 --- /dev/null +++ b/docs/source/examples/containers/inference_redirects.rst @@ -0,0 +1,19 @@ +Following redirects inside a deployment +======================================= + +By default, ``InferenceClient`` does not follow HTTP redirects. A 3xx response raises ``InferenceClientError``, and the error message shows the ``Location`` value. + +Set ``follow_redirects=True`` if your container sends redirects inside its own deployment. The client then follows a redirect only if all of these are true: + +- The target has the same scheme, host and port as ``endpoint_base_url``. +- The target path is the deployment path, or it starts with the deployment path and ``/``. +- The target path passes the same check as ``path``. +- The client followed fewer than 5 redirects. + +The client raises ``InferenceClientError`` for any other redirect. ``AsyncInferenceExecution.status_json()`` and ``result()`` never follow a redirect. + +``Deployment.run_sync`` and ``Deployment.run`` always use the default. To follow redirects, create an ``InferenceClient`` with the deployment's ``endpoint_base_url``, as this example shows. + +.. literalinclude:: ../../../../examples/containers/calling_the_endpoint_with_redirects.py + :language: python + :caption: Following redirects inside a deployment diff --git a/examples/containers/calling_the_endpoint_asynchronously.py b/examples/containers/calling_the_endpoint_asynchronously.py index e8af1ef..af327e3 100644 --- a/examples/containers/calling_the_endpoint_asynchronously.py +++ b/examples/containers/calling_the_endpoint_asynchronously.py @@ -54,7 +54,13 @@ headers=header, ) -while response.status() != AsyncStatus.Completed: +# Poll for status until the job completes or fails +while response.status() not in (AsyncStatus.Completed, AsyncStatus.Error): print(response.status_json()) sleep(1) -print(response.output()) + +if response.status() == AsyncStatus.Error: + # The status json has an Error field with the reason + print(f'Inference failed: {response.status_json()}') +else: + print(response.output()) diff --git a/examples/containers/calling_the_endpoint_with_inference_key_async.py b/examples/containers/calling_the_endpoint_with_inference_key_async.py index 44509f1..c5ce944 100644 --- a/examples/containers/calling_the_endpoint_with_inference_key_async.py +++ b/examples/containers/calling_the_endpoint_with_inference_key_async.py @@ -40,10 +40,14 @@ # Run the request asynchronously using the inference client async_inference_execution = inference_client.run(data=data, path='v1/completions') -# Poll for status until completion -while async_inference_execution.status() != AsyncStatus.Completed: +# Poll for status until the job completes or fails +while async_inference_execution.status() not in (AsyncStatus.Completed, AsyncStatus.Error): print(async_inference_execution.status_json()) sleep(5) -# Print the response -print(async_inference_execution.output()) +if async_inference_execution.status() == AsyncStatus.Error: + # The status json has an Error field with the reason + print(f'Inference failed: {async_inference_execution.status_json()}') +else: + # Print the response + print(async_inference_execution.output()) diff --git a/examples/containers/calling_the_endpoint_with_redirects.py b/examples/containers/calling_the_endpoint_with_redirects.py new file mode 100644 index 0000000..8f0ac0b --- /dev/null +++ b/examples/containers/calling_the_endpoint_with_redirects.py @@ -0,0 +1,60 @@ +# Copyright 2026 Verda Cloud Oy +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +from verda import VerdaClient +from verda.inference_client import InferenceClient, InferenceClientError + +# Configuration - replace with your deployment name +DEPLOYMENT_NAME = os.environ.get('VERDA_DEPLOYMENT_NAME') + +# Client id and secret authenticate calls to the Verda API, such as the deployment lookup +CLIENT_ID = os.environ.get('VERDA_CLIENT_ID') +CLIENT_SECRET = os.environ.get('VERDA_CLIENT_SECRET') + +# The inference key authenticates calls to the deployment endpoint +INFERENCE_KEY = os.environ.get('VERDA_INFERENCE_KEY') + +# Verda client instance, used only to look up the deployment +verda = VerdaClient(CLIENT_ID, CLIENT_SECRET) + +# Get the deployment, to read its endpoint base url +deployment = verda.containers.get_deployment_by_name(DEPLOYMENT_NAME) + +# By default the inference client does not follow redirects, and a 3xx response +# raises InferenceClientError. With follow_redirects=True, the client follows a +# redirect only if the target is inside this deployment: the same scheme, host and +# port, and a path under endpoint_base_url. It follows at most 5 redirects. +with InferenceClient( + inference_key=INFERENCE_KEY, + endpoint_base_url=deployment.endpoint_base_url, + follow_redirects=True, +) as inference_client: + # Make a synchronous request to the endpoint. + # This example demonstrates calling a SGLang deployment which serves LLMs using an OpenAI-compatible API format + data = { + 'model': 'deepseek-ai/deepseek-llm-7b-chat', + 'prompt': 'Write a haiku about the sea.', + 'max_tokens': 64, + 'temperature': 0.7, + } + + try: + response = inference_client.run_sync(data=data, path='v1/completions') + print(response.output()) + except InferenceClientError as e: + # A redirect that fails the follow_redirects check is not followed. The error + # message shows the Location value, and a hint if only a trailing slash differs. + print(f'Request failed: {e}') diff --git a/tests/unit_tests/authentication/test_authentication.py b/tests/unit_tests/authentication/test_authentication.py index 2d50d86..845501f 100644 --- a/tests/unit_tests/authentication/test_authentication.py +++ b/tests/unit_tests/authentication/test_authentication.py @@ -242,3 +242,38 @@ def test_is_expired(self, authentication_service): # assert assert is_expired_current assert not is_expired_future + + @pytest.mark.parametrize('status', [302, 307, 308]) + def test_authenticate_redirect_failed(self, authentication_service, endpoint, status): + # arrange + target = BASE_URL + '/moved' + responses.add(responses.POST, endpoint, status=status, headers={'Location': target}) + responses.add(responses.POST, target, json={}) + responses.add(responses.GET, target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + authentication_service.authenticate() + + # assert + assert excinfo.value.code is None + assert f'unexpected redirect ({status})' in excinfo.value.message + assert len(responses.calls) == 1 + + @pytest.mark.parametrize('status', [302, 307, 308]) + def test_refresh_redirect_failed(self, authentication_service, endpoint, status): + # arrange + authentication_service._refresh_token = REFRESH_TOKEN + target = BASE_URL + '/moved' + responses.add(responses.POST, endpoint, status=status, headers={'Location': target}) + responses.add(responses.POST, target, json={}) + responses.add(responses.GET, target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + authentication_service.refresh() + + # assert + assert excinfo.value.code is None + assert f'unexpected redirect ({status})' in excinfo.value.message + assert len(responses.calls) == 1 diff --git a/tests/unit_tests/http_client/test_http_client.py b/tests/unit_tests/http_client/test_http_client.py index cb80681..00a075f 100644 --- a/tests/unit_tests/http_client/test_http_client.py +++ b/tests/unit_tests/http_client/test_http_client.py @@ -17,10 +17,11 @@ from unittest.mock import Mock import pytest +import requests import responses # https://github.com/getsentry/responses from verda.exceptions import APIException -from verda.http_client._http_client import _encode_path_segment +from verda.http_client._http_client import _encode_path_segment, handle_error INVALID_REQUEST = 'invalid_request' INVALID_REQUEST_MESSAGE = 'Your existence is invalid' @@ -497,3 +498,86 @@ def test_delete_failed(self, http_client): # assert assert excinfo.value.code == INVALID_REQUEST assert excinfo.value.message == INVALID_REQUEST_MESSAGE + + @pytest.mark.parametrize('status', [301, 302, 303, 307, 308]) + @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) + def test_request_methods_redirect_failed(self, http_client, method, status): + # arrange + url = http_client._base_url + '/test' + target = http_client._base_url + '/moved' + responses.add(method.upper(), url, status=status, headers={'Location': target}) + responses.add(method.upper(), target, json={}) + responses.add(responses.GET, target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + getattr(http_client, method)('/test') + + # assert + assert excinfo.value.code is None + assert f'unexpected redirect ({status})' in excinfo.value.message + assert len(responses.calls) == 1 + + @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) + def test_request_methods_allow_redirects_false_successful(self, http_client, method): + # arrange + url = http_client._base_url + '/test' + responses.add(method.upper(), url, json={}) + + # act + response = getattr(http_client, method)('/test', allow_redirects=False) + + # assert + assert response.status_code == 200 + assert len(responses.calls) == 1 + + @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) + def test_request_methods_allow_redirects_true_redirect_failed(self, http_client, method): + # arrange + url = http_client._base_url + '/test' + target = http_client._base_url + '/moved' + responses.add(method.upper(), url, status=307, headers={'Location': target}) + responses.add(method.upper(), target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + getattr(http_client, method)('/test', allow_redirects=True) + + # assert + assert excinfo.value.code is None + assert 'unexpected redirect (307)' in excinfo.value.message + assert len(responses.calls) == 1 + + +def test_handle_error_redirect_failed(): + # arrange + response = requests.Response() + response.status_code = 302 + response.headers['Location'] = 'https://api.example.com/v1/moved' + + # act + with pytest.raises(APIException) as excinfo: + handle_error(response) + + # assert + assert excinfo.value.code is None + assert excinfo.value.message == ( + "unexpected redirect (302) to 'https://api.example.com/v1/moved'; redirects are not followed" + ) + + +@pytest.mark.parametrize( + ('status', 'headers'), + [(304, {}), (304, {'Location': 'https://api.example.com/v1/moved'}), (300, {}), (307, {})], +) +def test_handle_error_non_redirect_3xx_successful(status, headers): + # arrange + response = requests.Response() + response.status_code = status + response.headers.update(headers) + + # act + handle_error(response) + + # assert + assert response.status_code == status diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index ca06799..2fcfb42 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -13,21 +13,70 @@ # limitations under the License. import pytest +import requests import responses # https://github.com/getsentry/responses from responses import matchers -from verda.inference_client import InferenceClient, InferenceClientError +from verda.inference_client import ( + AsyncInferenceExecution, + AsyncStatus, + InferenceClient, + InferenceClientError, +) BASE_URL = 'https://inference.example.com/v1/my-deployment' VERBS = ['get', 'post', 'put', 'delete', 'patch', 'head', 'options'] +PREDICT_URL = f'{BASE_URL}/predict' +STATUS_URL = 'https://inference.example.com/v1/status/my-deployment' +RESULT_URL = 'https://inference.example.com/v1/result/my-deployment' +REDIRECT_URL = 'https://inference.example.com/v1/moved' +MOVED_URL = f'{BASE_URL}/moved' +EXECUTION_ID = 'exec-1' + +REDIRECT_STATUSES = [301, 302, 303, 307, 308] + + +def _execution(client): + return AsyncInferenceExecution(client, EXECUTION_ID, AsyncStatus.Initialized) + + +# (method, url the call sends to, call) +ENTRY_POINTS = { + 'run_sync': ('POST', PREDICT_URL, lambda c: c.run_sync({'a': 1}, path='predict')), + 'run': ('POST', PREDICT_URL, lambda c: c.run({'a': 1}, path='predict')), + 'run_no_response': ( + 'POST', + PREDICT_URL, + lambda c: c.run({'a': 1}, path='predict', no_response=True), + ), + 'get': ('GET', PREDICT_URL, lambda c: c.get('predict')), + 'post': ('POST', PREDICT_URL, lambda c: c.post('predict', json={'a': 1})), + 'put': ('PUT', PREDICT_URL, lambda c: c.put('predict', json={'a': 1})), + 'delete': ('DELETE', PREDICT_URL, lambda c: c.delete('predict')), + 'patch': ('PATCH', PREDICT_URL, lambda c: c.patch('predict', json={'a': 1})), + 'head': ('HEAD', PREDICT_URL, lambda c: c.head('predict')), + 'options': ('OPTIONS', PREDICT_URL, lambda c: c.options('predict')), + 'health': ('GET', f'{BASE_URL}/health', lambda c: c.health()), + 'status_json': ('GET', STATUS_URL, lambda c: _execution(c).status_json()), + 'result': ('GET', RESULT_URL, lambda c: _execution(c).result()), +} + + @pytest.fixture def inference_client(): return InferenceClient(inference_key='key-123', endpoint_base_url=BASE_URL) +@pytest.fixture +def following_client(): + return InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=True + ) + + class TestBuildUrl: """`path` is a caller-chosen multi-segment path, but it must stay under the base.""" @@ -235,3 +284,584 @@ def test_base_url_without_a_deployment_path_is_rejected(self, base_url): def test_the_deployment_name_is_sliced_out_correctly(self, inference_client): assert inference_client.deployment_name == 'my-deployment' assert inference_client.base_domain == 'https://inference.example.com/v1' + + +class TestRedirectResponses: + """A 3xx response is reported to the caller as an error.""" + + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + @pytest.mark.parametrize('entry_point', list(ENTRY_POINTS)) + def test_request_redirect_failed(self, inference_client, entry_point, status): + # arrange + method, url, call = ENTRY_POINTS[entry_point] + responses.add(method, url, status=status, headers={'Location': REDIRECT_URL}) + for target_method in ('GET', method): + responses.add(target_method, REDIRECT_URL, json={'Id': EXECUTION_ID}) + + # act + with pytest.raises(InferenceClientError, match=rf'redirect \({status}\)'): + call(inference_client) + + # assert + assert len(responses.calls) == 1 + assert responses.calls[0].request.url == url + + def test_request_redirect_to_another_host_failed(self, inference_client): + # arrange + inference_client.set_global_header('X-Request-Source', 'tests') + target = 'https://other.example.net/moved' + responses.add(responses.POST, PREDICT_URL, status=307, headers={'Location': target}) + responses.add(responses.POST, target, json={}) + + # act + with pytest.raises(InferenceClientError, match='redirects are not followed'): + inference_client.run_sync({'a': 1}, path='predict') + + # assert + assert len(responses.calls) == 1 + + def test_session_redirect_returned_as_is_successful(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': REDIRECT_URL}) + responses.add(responses.GET, REDIRECT_URL, json={}) + + # act + response = inference_client._session.get(PREDICT_URL, allow_redirects=True) + + # assert + assert response.status_code == 307 + assert len(responses.calls) == 1 + + def test_request_redirect_inside_deployment_by_default_failed(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, json={}) + + # act + with pytest.raises(InferenceClientError, match='redirects are not followed'): + inference_client.get('predict') + + # assert + assert inference_client.follow_redirects is False + assert len(responses.calls) == 1 + + @pytest.mark.parametrize( + ('path', 'location', 'hint'), + [ + ('predict', f'{PREDICT_URL}/', 'add a trailing slash to `path`'), + ('predict', '/predict/', 'add a trailing slash to `path`'), + ('predict/', PREDICT_URL, 'remove the trailing slash from `path`'), + ('predict/', '/predict', 'remove the trailing slash from `path`'), + ], + ) + def test_request_redirect_trailing_slash_failed(self, inference_client, path, location, hint): + # arrange + responses.add( + responses.GET, f'{BASE_URL}/{path}', status=307, headers={'Location': location} + ) + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.get(path) + + # assert + assert str(excinfo.value).endswith(hint) + + def test_request_redirect_to_another_path_has_no_hint_failed(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': MOVED_URL}) + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.get('predict') + + # assert + assert str(excinfo.value).endswith('redirects are not followed') + + @pytest.mark.parametrize( + ('status', 'headers'), + [ + (304, {}), + (304, {'Location': MOVED_URL}), + (300, {}), + (307, {}), + ], + ) + @pytest.mark.parametrize('follow_redirects', [False, True]) + def test_request_non_redirect_3xx_successful(self, follow_redirects, status, headers): + # arrange + client = InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=follow_redirects + ) + responses.add(responses.GET, PREDICT_URL, status=status, headers=headers) + + # act + response = client.get('predict') + + # assert + assert response.status_code == status + assert len(responses.calls) == 1 + + @pytest.mark.parametrize( + 'location', + [ + 'https://[invalid', + 'https://[invalid/v1/my-deployment/moved', + 'https://inference.example.com:99999/v1/my-deployment/moved', + ], + ) + @pytest.mark.parametrize('follow_redirects', [False, True]) + def test_request_redirect_malformed_location_failed(self, follow_redirects, location): + # arrange + client = InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=follow_redirects + ) + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': location}) + + # act + with pytest.raises(InferenceClientError, match=r'redirect \(307\)'): + client.get('predict') + + # assert + assert len(responses.calls) == 1 + + +class TestFollowRedirects: + """With `follow_redirects=True`, only a redirect inside the deployment is followed.""" + + @pytest.mark.parametrize( + ('location', 'target'), + [ + (MOVED_URL, MOVED_URL), + ('/v1/my-deployment/moved', MOVED_URL), + ('moved', MOVED_URL), + ('/v1/my-deployment/a/b?q=1', f'{BASE_URL}/a/b?q=1'), + ('/v1/my-deployment', BASE_URL), + ('/v1/my-deployment/v1/models/m:predict', f'{BASE_URL}/v1/models/m:predict'), + ], + ) + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + def test_follow_redirect_inside_deployment_successful( + self, following_client, status, location, target + ): + # arrange + responses.add(responses.GET, PREDICT_URL, status=status, headers={'Location': location}) + responses.add( + responses.GET, + target, + json={'out': 1}, + match=[matchers.header_matcher({'Authorization': 'Bearer key-123'})], + ) + + # act + response = following_client.get('predict') + + # assert + assert response.json() == {'out': 1} + assert len(responses.calls) == 2 + assert responses.calls[1].request.url == target + + @pytest.mark.parametrize('status', [307, 308]) + def test_follow_redirect_keeps_method_and_body_successful(self, following_client, status): + # arrange + responses.add(responses.POST, PREDICT_URL, status=status, headers={'Location': MOVED_URL}) + responses.add( + responses.POST, + MOVED_URL, + json={'out': 1}, + match=[matchers.json_params_matcher({'a': 1})], + ) + + # act + response = following_client.run_sync({'a': 1}, path='predict') + + # assert + assert response.output() == {'out': 1} + assert responses.calls[1].request.method == 'POST' + + @pytest.mark.parametrize('status', [301, 302, 303]) + def test_follow_redirect_changes_post_to_get_successful(self, following_client, status): + # arrange + responses.add(responses.POST, PREDICT_URL, status=status, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, json={'out': 1}) + + # act + response = following_client.run_sync({'a': 1}, path='predict') + + # assert + assert response.output() == {'out': 1} + assert responses.calls[1].request.method == 'GET' + assert responses.calls[1].request.body is None + + @pytest.mark.parametrize( + 'location', + [ + # another deployment + 'https://inference.example.com/v1/other-deployment/moved', + '/v1/other-deployment/moved', + '/v1/moved', + # a deployment whose name starts with this one + '/v1/my-deployment-2/moved', + 'https://inference.example.com/v1/my-deployment-2', + # relative path segments, plain and encoded + '../other-deployment/moved', + '/v1/my-deployment/../other-deployment/moved', + '/v1/my-deployment/%2e%2e/other-deployment/moved', + '/v1/my-deployment/%2E%2E/other-deployment/moved', + '/v1/my-deployment/..%2Fother-deployment/moved', + '/v1/my-deployment/a/..%2F..%2F..%2Fother-deployment', + '/v1/my-deployment/./moved', + '/v1/my-deployment/..;/other-deployment/moved', + '/v1/my-deployment/..%252f..%252fother-deployment', + # characters that `path` does not allow + '/v1/my-deployment/a%20b', + '/v1/my-deployment/a;b', + '/v1/my-deployment/a\\b', + # another host + 'https://other.example.net/v1/my-deployment/moved', + '//other.example.net/v1/my-deployment/moved', + # another scheme + 'http://inference.example.com/v1/my-deployment/moved', + # another port + 'https://inference.example.com:8443/v1/my-deployment/moved', + 'https://inference.example.com:bad/v1/my-deployment/moved', + # user info + 'https://user@inference.example.com/v1/my-deployment/moved', + ], + ) + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + def test_follow_redirect_outside_deployment_failed(self, following_client, status, location): + # arrange + responses.add(responses.POST, PREDICT_URL, status=status, headers={'Location': location}) + + # act + with pytest.raises(InferenceClientError, match=rf'redirect \({status}\)') as excinfo: + following_client.run_sync({'a': 1}, path='predict') + + # assert + assert 'failed the follow_redirects check' in str(excinfo.value) + assert len(responses.calls) == 1 + + def test_follow_redirect_stops_at_rejected_hop_failed(self, following_client): + # arrange + location = 'https://other.example.net/moved' + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, status=307, headers={'Location': location}) + + # act + with pytest.raises(InferenceClientError, match=location): + following_client.get('predict') + + # assert + assert len(responses.calls) == 2 + + def test_follow_redirect_hop_limit_failed(self, following_client): + # arrange + responses.add( + responses.GET, PREDICT_URL, status=307, headers={'Location': f'{BASE_URL}/hop-1'} + ) + for hop in range(1, 10): + responses.add( + responses.GET, + f'{BASE_URL}/hop-{hop}', + status=307, + headers={'Location': f'{BASE_URL}/hop-{hop + 1}'}, + ) + + # act + with pytest.raises(InferenceClientError, match='Exceeded 5 redirects'): + following_client.get('predict') + + # assert + assert len(responses.calls) == 6 + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + def test_async_request_redirect_failed(self, following_client, entry_point, status): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, status=status, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, json={'Status': 'Completed'}) + + # act + with pytest.raises(InferenceClientError, match='redirects are not followed'): + call(following_client) + + # assert + assert len(responses.calls) == 1 + + def test_session_redirect_outside_deployment_returned_as_is_successful(self, following_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': REDIRECT_URL}) + responses.add(responses.GET, REDIRECT_URL, json={}) + + # act + response = following_client._session.get(PREDICT_URL, allow_redirects=True) + + # assert + assert response.status_code == 307 + assert len(responses.calls) == 1 + + +class TestRequests: + """Successful inference requests.""" + + def test_run_sync_successful(self, inference_client): + # arrange + responses.add( + responses.POST, + PREDICT_URL, + json={'out': 1}, + match=[ + matchers.json_params_matcher({'a': 1}), + matchers.header_matcher({'Authorization': 'Bearer key-123', 'X-Extra': 'x'}), + ], + ) + + # act + response = inference_client.run_sync({'a': 1}, path='predict', headers={'X-Extra': 'x'}) + + # assert + assert response.status_code == 200 + assert response.output() == {'out': 1} + + def test_run_sync_stream_successful(self, inference_client): + # arrange + responses.add( + responses.POST, + PREDICT_URL, + body='line 1\nline 2\n', + content_type='text/event-stream', + ) + + # act + response = inference_client.run_sync({'a': 1}, path='predict', stream=True) + + # assert + assert list(response.stream()) == [b'line 1', b'line 2'] + + def test_run_then_status_and_result_successful(self, inference_client): + # arrange + responses.add( + responses.POST, + PREDICT_URL, + json={'Id': EXECUTION_ID}, + match=[matchers.header_matcher({'Prefer': 'respond-async'})], + ) + inference_id = matchers.header_matcher( + {'Authorization': 'Bearer key-123', 'X-Inference-Id': EXECUTION_ID} + ) + responses.add(responses.GET, STATUS_URL, json={'Status': 'Completed'}, match=[inference_id]) + responses.add(responses.GET, RESULT_URL, json={'out': 1}, match=[inference_id]) + + # act + execution = inference_client.run({'a': 1}, path='predict') + status = execution.status_json() + result = execution.result() + + # assert + assert execution.id == EXECUTION_ID + assert status == {'Status': 'Completed'} + assert execution.status() == AsyncStatus.Completed + assert result == {'out': 1} + + def test_result_text_successful(self, inference_client): + # arrange + responses.add(responses.GET, RESULT_URL, body='plain', content_type='text/plain') + + # act + result = _execution(inference_client).result() + + # assert + assert result == {'result': 'plain'} + + @pytest.mark.parametrize( + 'content_type', ['application/json; charset=utf-8', 'Application/JSON', 'application/json'] + ) + def test_result_json_content_type_successful(self, inference_client, content_type): + # arrange + responses.add(responses.GET, RESULT_URL, body='{"out": 1}', content_type=content_type) + + # act + result = _execution(inference_client).result() + + # assert + assert result == {'out': 1} + + def test_result_without_content_type_successful(self, inference_client): + # arrange + responses.add(responses.GET, RESULT_URL, body='plain', content_type=None) + + # act + result = _execution(inference_client).result() + + # assert + assert 'Content-Type' not in responses.calls[0].response.headers + assert result == {'result': 'plain'} + + def test_status_json_error_successful(self, inference_client): + # arrange + status = { + 'Id': EXECUTION_ID, + 'Status': 'Error', + 'Error': {'Message': 'inference returned non-200 status code 500'}, + } + responses.add(responses.GET, STATUS_URL, json=status) + execution = _execution(inference_client) + + # act + response_json = execution.status_json() + + # assert + assert response_json == status + assert execution.status() == AsyncStatus.Error + + def test_result_of_failed_job_failed(self, inference_client): + # arrange + responses.add( + responses.GET, + RESULT_URL, + status=500, + body='model crashed: out of memory', + content_type='text/plain', + ) + + # act + with pytest.raises(InferenceClientError) as excinfo: + _execution(inference_client).result() + + # assert + assert '500 Server Error' in str(excinfo.value) + assert str(excinfo.value).endswith(': model crashed: out of memory') + + def test_result_before_completion_failed(self, inference_client): + # arrange + responses.add( + responses.GET, RESULT_URL, status=404, json={'msg': 'inference result not found'} + ) + + # act + with pytest.raises(InferenceClientError) as excinfo: + _execution(inference_client).result() + + # assert + assert '404 Client Error' in str(excinfo.value) + assert 'inference result not found' in str(excinfo.value) + + def test_request_long_error_body_failed(self, inference_client): + # arrange + responses.add(responses.POST, PREDICT_URL, status=500, body='x' * 1000) + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.run_sync({'a': 1}, path='predict') + + # assert + assert str(excinfo.value).endswith(': ' + 'x' * 500 + '...') + + def test_request_empty_error_body_failed(self, inference_client): + # arrange + responses.add(responses.POST, PREDICT_URL, status=500, body='') + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.run_sync({'a': 1}, path='predict') + + # assert + assert str(excinfo.value).endswith(f'for url: {PREDICT_URL}') + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + def test_async_requests_client_timeout_successful(self, inference_client, entry_point): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, json={'Status': 'Completed'}) + + # act + call(inference_client) + + # assert + assert responses.calls[0].request.req_kwargs['timeout'] == inference_client.timeout_seconds + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + def test_async_requests_failed(self, inference_client, entry_point): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, status=500, json={'error': 'boom'}) + + # act + with pytest.raises(InferenceClientError, match='500'): + call(inference_client) + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + def test_async_requests_timeout_failed(self, inference_client, entry_point): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, body=requests.exceptions.ReadTimeout()) + + # act + with pytest.raises(InferenceClientError, match='timed out'): + call(inference_client) + + @pytest.mark.parametrize( + 'entry_point', ['get', 'post', 'put', 'delete', 'patch', 'head', 'options'] + ) + def test_http_method_helpers_successful(self, inference_client, entry_point): + # arrange + method, url, call = ENTRY_POINTS[entry_point] + responses.add(method, url, status=200) + + # act + response = call(inference_client) + + # assert + assert response.status_code == 200 + assert len(responses.calls) == 1 + + @pytest.mark.parametrize( + 'entry_point', ['get', 'post', 'put', 'delete', 'patch', 'head', 'options', 'health'] + ) + def test_http_method_helpers_client_timeout_successful(self, inference_client, entry_point): + # arrange + method, url, call = ENTRY_POINTS[entry_point] + responses.add(method, url, status=200) + + # act + call(inference_client) + + # assert + assert responses.calls[0].request.req_kwargs['timeout'] == inference_client.timeout_seconds + + def test_http_method_helpers_explicit_timeout_successful(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=200) + + # act + inference_client.get('predict', timeout_seconds=7) + + # assert + assert responses.calls[0].request.req_kwargs['timeout'] == 7 + + def test_get_with_params_successful(self, inference_client): + # arrange + responses.add( + responses.GET, + PREDICT_URL, + json={}, + match=[matchers.query_param_matcher({'q': 'x'})], + ) + + # act + response = inference_client.get('predict', params={'q': 'x'}) + + # assert + assert response.status_code == 200 + + def test_health_successful(self, inference_client): + # arrange + responses.add(responses.GET, f'{BASE_URL}/health', json={'ok': True}) + + # act + response = inference_client.health() + + # assert + assert response.json() == {'ok': True} diff --git a/verda/authentication/_authentication.py b/verda/authentication/_authentication.py index a88efb0..d23b78e 100644 --- a/verda/authentication/_authentication.py +++ b/verda/authentication/_authentication.py @@ -54,7 +54,9 @@ def authenticate(self) -> dict: 'client_secret': self._client_secret, } - response = requests.post(url, json=payload, headers=self._generate_headers()) + response = requests.post( + url, json=payload, headers=self._generate_headers(), allow_redirects=False + ) handle_error(response) auth_data = response.json() @@ -87,7 +89,9 @@ def refresh(self) -> dict: payload = {'grant_type': REFRESH_TOKEN, 'refresh_token': self._refresh_token} - response = requests.post(url, json=payload, headers=self._generate_headers()) + response = requests.post( + url, json=payload, headers=self._generate_headers(), allow_redirects=False + ) # if refresh token is also expired, authenticate again: if response.status_code == 401 or response.status_code == 400: diff --git a/verda/http_client/_http_client.py b/verda/http_client/_http_client.py index 86bf149..4323c10 100644 --- a/verda/http_client/_http_client.py +++ b/verda/http_client/_http_client.py @@ -70,7 +70,12 @@ def _encode_path_segment(value: object) -> str: def handle_error(response: requests.Response) -> None: - """Checks for the response status code and raises an exception if it's 400 or higher. + """Checks the response and raises an exception for a redirect or an error status. + + Requests are sent with ``allow_redirects=False``, so a redirect arrives here + unfollowed and is an error: ``response.ok`` alone would return it to the caller as + a success. A redirect is what ``requests`` would follow: 301, 302, 303, 307 or 308 + with a ``Location`` header. Any other 3xx, such as 304, is returned unchanged. Args: response: The API call response. @@ -78,6 +83,13 @@ def handle_error(response: requests.Response) -> None: Raises: APIException: An api exception with message and error type code. """ + if response.is_redirect: + location = response.headers['Location'] + raise APIException( + None, + f'unexpected redirect ({response.status_code}) to {location!r}; ' + 'redirects are not followed', + ) if not response.ok: data = json.loads(response.text) code = data['code'] if 'code' in data else None @@ -291,6 +303,7 @@ def _request( params: Dictionary of querystring data to attach to the request. path_params: Values substituted into ``{name}`` placeholders in the url. **kwargs: Additional keyword arguments passed through to ``requests``. + ``allow_redirects`` is ignored: redirects are never followed. Returns: The response object. @@ -299,13 +312,25 @@ def _request( ValueError: If the url and ``path_params`` do not produce a safe path. APIException: An api exception with message and error type code. """ + # Accepted for compatibility but never passed on: True would follow a redirect, + # and False beside our own keyword would raise TypeError. + kwargs.pop('allow_redirects', None) + # Validate before the refresh so a rejected name costs no auth round-trip. url = self._add_base_url(self._build_path(url, path_params)) self._refresh_token_if_expired() + # A followed same-host redirect keeps the bearer token, method and body, and + # its target never went through the path validation above. response = requests.request( - method, url, json=json, headers=self._generate_headers(), params=params, **kwargs + method, + url, + json=json, + headers=self._generate_headers(), + params=params, + allow_redirects=False, + **kwargs, ) handle_error(response) diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index 6ed8561..872439b 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -17,7 +17,7 @@ from dataclasses import dataclass from enum import Enum from typing import Any -from urllib.parse import urlparse +from urllib.parse import urljoin, urlparse, urlsplit import requests from dataclasses_json import Undefined, dataclass_json # type: ignore @@ -43,6 +43,9 @@ class AsyncStatus(str, Enum): Queue = 'Queue' Inference = 'Inference' Completed = 'Completed' + # The job finished without a result, for example because the deployment + # returned an error status or could not be reached. + Error = 'Error' @dataclass_json(undefined=Undefined.EXCLUDE) @@ -124,11 +127,167 @@ def stream(self, chunk_size: int = 512, as_text: bool = True) -> Generator[Any, yield chunk +# The most characters of a response body that an HTTP error message includes. +_MAX_ERROR_BODY = 500 + +# The most redirects a deployment request follows when `follow_redirects` is on. +_MAX_REDIRECTS = 5 + +_DEFAULT_PORTS = {'http': 80, 'https': 443} + + +def _origin(url: str) -> tuple[str, str | None, int | None, str | None, str | None] | None: + """Return the scheme, host, port and user info of a url, or None if the port is invalid.""" + parts = urlsplit(url) + scheme = parts.scheme.lower() + try: + port = parts.port or _DEFAULT_PORTS.get(scheme) + except ValueError: + return None + return scheme, parts.hostname, port, parts.username, parts.password + + +def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: str) -> bool: + """Whether a redirect ``Location`` stays inside the deployment. + + The target must have the same scheme, host and port as ``endpoint_base_url``, and + its path must be the deployment path or start with ``/``. The raw + ``Location`` path must have no relative segment: ``requests`` decodes ``%2E`` before + it resolves the target, so a prefix check on the resolved url alone misses + ``//%2E%2E/``. The rest of the target path must pass the same + check as ``path`` in ``_build_url``, which also refuses forms such as ``..;/`` and + ``..%252f``. The query string is not checked: it does not change the target path. + A ``Location`` that cannot be parsed is not followed. + + Args: + endpoint_base_url: The deployment's base url, without a trailing slash. + response_url: The url of the request that returned the redirect. + location: The ``Location`` header value. + + Returns: + True if the redirect may be followed. + """ + try: + if has_relative_path_segment(urlsplit(location).path): + return False + target = urljoin(response_url, location) + target_path = urlsplit(target).path + except ValueError: + return False + base_origin = _origin(endpoint_base_url) + if base_origin is None or _origin(target) != base_origin: + return False + deployment_path = urlsplit(endpoint_base_url).path + if target_path != deployment_path and not target_path.startswith(deployment_path + '/'): + return False + try: + check_request_path(target_path[len(deployment_path) :], _ALLOWED_IN_INFERENCE_PATH) + except ValueError: + return False + return True + + +def _error_body(response: requests.Response | None) -> str: + """Return the start of an error response body, to append to the error message. + + For a failed async job, ``/result/`` returns the deployment's own error status and + body. Without the body, the caller cannot see why the inference failed. + + Args: + response: The response with the HTTP error status, if any. + + Returns: + ``': '``, cut to ``_MAX_ERROR_BODY`` characters, or an empty string. + """ + if response is None: + return '' + try: + body = response.text.strip() + except requests.exceptions.RequestException: + return '' + if not body: + return '' + if len(body) > _MAX_ERROR_BODY: + body = body[:_MAX_ERROR_BODY] + '...' + return f': {body}' + + +def _trailing_slash_hint(response_url: str, location: str | None) -> str: + """Suggest a ``path`` fix when a redirect only adds or removes a trailing slash. + + The ``Location`` may lack the deployment prefix, so the target path only has to be + a suffix of the request path, ignoring the trailing slash. + + Args: + response_url: The url of the request that returned the redirect. + location: The ``Location`` header value, if any. + + Returns: + The hint to append to the error message, or an empty string. It is empty if + the ``Location`` cannot be parsed. + """ + if not location: + return '' + try: + request_path = urlsplit(response_url).path + target_path = urlsplit(urljoin(response_url, location)).path + except ValueError: + return '' + stripped = target_path.rstrip('/') + if not stripped or request_path.endswith('/') == target_path.endswith('/'): + return '' + if not request_path.rstrip('/').endswith(stripped): + return '' + if target_path.endswith('/'): + return '; add a trailing slash to `path`' + return '; remove the trailing slash from `path`' + + +class _RedirectPolicySession(requests.Session): + """A session that follows a redirect only inside the deployment, and only if enabled. + + Requests are sent with ``allow_redirects=False`` unless the client enables + ``follow_redirects``. This is the backstop for a call that forgets: without an + ``endpoint_base_url`` no redirect target is reported, so the 3xx response is + returned as-is instead of being followed. + """ + + def __init__(self, endpoint_base_url: str | None = None) -> None: + """Initialize the session. + + Args: + endpoint_base_url: The deployment's base url. If set, a redirect inside + the deployment is followed, up to ``_MAX_REDIRECTS`` times. + """ + super().__init__() + self.endpoint_base_url = endpoint_base_url + self.max_redirects = _MAX_REDIRECTS + + def get_redirect_target(self, resp: requests.Response) -> str | None: + """Report the redirect target only if it stays inside the deployment. + + ``resolve_redirects`` stops when this returns None, and the 3xx response is + returned to the caller. + """ + if self.endpoint_base_url is None: + return None + location = super().get_redirect_target(resp) + if location is None or not _is_inside_deployment( + self.endpoint_base_url, resp.url, location + ): + return None + return location + + class InferenceClient: """Inference client.""" def __init__( - self, inference_key: str, endpoint_base_url: str, timeout_seconds: int = 60 * 5 + self, + inference_key: str, + endpoint_base_url: str, + timeout_seconds: int = 60 * 5, + follow_redirects: bool = False, ) -> None: """Initialize the InferenceClient. @@ -136,6 +295,10 @@ def __init__( inference_key: The authentication key for the API endpoint_base_url: The base URL for the API timeout_seconds: Request timeout in seconds + follow_redirects: Follow a redirect whose target is inside this deployment: + same scheme, host and port, and a path under ``endpoint_base_url``. + Any other redirect raises ``InferenceClientError``. Async status and + result requests never follow a redirect. Defaults to False. Raises: InferenceClientError: If the parameters are invalid @@ -167,7 +330,8 @@ def __init__( self.base_domain = self.endpoint_base_url[: self.endpoint_base_url.rindex('/')] self.deployment_name = self.endpoint_base_url[self.endpoint_base_url.rindex('/') + 1 :] self.timeout_seconds = timeout_seconds - self._session = requests.Session() + self.follow_redirects = follow_redirects + self._session = _RedirectPolicySession(self.endpoint_base_url if follow_redirects else None) self._global_headers = { 'Authorization': f'Bearer {inference_key}', 'Content-Type': 'application/json', @@ -272,21 +436,87 @@ def _make_request(self, method: str, path: str, **kwargs) -> requests.Response: Raises: InferenceClientError: If the request fails """ - timeout = kwargs.pop('timeout_seconds', self.timeout_seconds) + # The HTTP verb helpers pass timeout_seconds=None when the caller sets none. + timeout = kwargs.pop('timeout_seconds', None) + if timeout is None: + timeout = self.timeout_seconds + return self._send( + method, + self._build_url(path), + path, + timeout, + follow_redirects=self.follow_redirects, + headers=self._build_request_headers(kwargs.pop('headers', None)), + **kwargs, + ) + + def _send( + self, + method: str, + url: str, + path: str, + timeout: float | None, + follow_redirects: bool = False, + **kwargs, + ) -> requests.Response: + """Send a request, following a redirect only inside the deployment. + + Every request carrying the inference key goes through here. By default a + redirect is not followed: ``requests`` keeps the ``Authorization`` header, + method and body on a same-host redirect, so a ``Location`` from the response + would retarget the request at a path that never went through ``_build_url``. + With ``follow_redirects``, the session follows only a target inside the + deployment, and any other redirect is returned here and raised. + + Args: + method: HTTP method to use + url: Full request URL + path: The path as the caller gave it, for error messages + timeout: Request timeout in seconds + follow_redirects: Follow a redirect whose target is inside the deployment + **kwargs: Additional arguments to pass to the request + + Returns: + Response object from the request + + Raises: + InferenceClientError: If the request fails or the response is a redirect + that is not followed + """ try: response = self._session.request( method=method, - url=self._build_url(path), - headers=self._build_request_headers(kwargs.pop('headers', None)), + url=url, timeout=timeout, + allow_redirects=follow_redirects, **kwargs, ) + # raise_for_status() lets a 3xx through. is_redirect is the rule requests + # uses to follow: 301, 302, 303, 307 or 308 with a Location header. Any + # other 3xx, such as 304 Not Modified, goes back to the caller. + if response.is_redirect: + location = response.headers['Location'] + response.close() + reason = ( + 'the target failed the follow_redirects check and was not followed' + if follow_redirects + else 'redirects are not followed' + ) + raise InferenceClientError( + f'Request to {path} returned a redirect ({response.status_code}) ' + f'to {location!r}; {reason}' + f'{_trailing_slash_hint(response.url, location)}' + ) response.raise_for_status() return response except requests.exceptions.Timeout as e: raise InferenceClientError( f'Request to {path} timed out after {timeout} seconds' ) from e + except requests.exceptions.HTTPError as e: + raise InferenceClientError( + f'Request to {path} failed: {e!s}{_error_body(e.response)}' + ) from e except requests.exceptions.RequestException as e: raise InferenceClientError(f'Request to {path} failed: {e!s}') from e @@ -546,21 +776,34 @@ def status(self) -> AsyncStatus: """ return self._status + def _get(self, url: str) -> requests.Response: + """GET an async status or result url for this execution. + + These urls belong to the gateway, not the deployment, so a redirect is never + followed, even if the client enables ``follow_redirects``. + """ + client = self._inference_client + return client._send( + 'GET', + url, + url, + client.timeout_seconds, + headers=client._build_request_headers({self.INFERENCE_ID_HEADER: self.id}), + ) + def status_json(self) -> dict[str, Any]: """Get the current status of the async inference execution. Return the status json. Returns: dict[str, Any]: The status response containing the execution status and other metadata + + Raises: + InferenceClientError: If the request fails """ url = ( f'{self._inference_client.base_domain}/status/{self._inference_client.deployment_name}' ) - response = self._inference_client._session.get( - url, - headers=self._inference_client._build_request_headers( - {self.INFERENCE_ID_HEADER: self.id} - ), - ) + response = self._get(url) response_json = response.json() self._status = AsyncStatus(response_json['Status']) @@ -572,18 +815,18 @@ def result(self) -> dict[str, Any]: Returns: dict[str, Any]: The results of the inference execution + + Raises: + InferenceClientError: If the request fails """ url = ( f'{self._inference_client.base_domain}/result/{self._inference_client.deployment_name}' ) - response = self._inference_client._session.get( - url, - headers=self._inference_client._build_request_headers( - {self.INFERENCE_ID_HEADER: self.id} - ), - ) + response = self._get(url) - if response.headers['Content-Type'] == 'application/json': + # The gateway and many servers send 'application/json; charset=utf-8'. + content_type = response.headers.get('Content-Type', '') + if content_type.split(';', 1)[0].strip().lower() == 'application/json': return response.json() else: return {'result': response.text}