From dc84a62b4c9195c2ad6207d8c1d745f965f5a526 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Wed, 29 Sep 2021 19:09:34 -0400 Subject: [PATCH] Implement HTTP Request Timeouts - Take 2 (#62) HTTP timeout implementation --- .github/workflows/codeql-analysis.yml | 64 +++---- .github/workflows/pythonpublish.yml | 32 ++-- qbittorrentapi/auth.py | 23 +-- qbittorrentapi/decorators.py | 51 +++--- qbittorrentapi/request.py | 239 ++++++++++++++++---------- qbittorrentapi/rss.py | 14 +- requirements-dev.txt | 2 +- tests/test_request.py | 55 +++++- 8 files changed, 289 insertions(+), 191 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 95af002a1..0451a893b 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -15,35 +15,35 @@ jobs: runs-on: ubuntu-latest steps: - - name: Checkout repository - uses: actions/checkout@v2 - with: - # We must fetch at least the immediate parents so that if this is - # a pull request then we can checkout the head. - fetch-depth: 2 - - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - # Override language selection by uncommenting this and choosing your languages - # with: - # languages: go, javascript, csharp, python, cpp, java - - # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). - # If this step fails, then you should remove it and run the build manually (see below) - - name: Autobuild - uses: github/codeql-action/autobuild@v1 - - # ℹī¸ Command-line programs to run using the OS shell. - # 📚 https://git.io/JvXDl - - # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines - # and modify them (or add more) to build your code if your project - # uses a compiled language - - #- run: | - # make bootstrap - # make release - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 + - name: Checkout repository + uses: actions/checkout@v2 + with: + # We must fetch at least the immediate parents so that if this is + # a pull request then we can checkout the head. + fetch-depth: 2 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + # Override language selection by uncommenting this and choosing your languages + # with: + # languages: go, javascript, csharp, python, cpp, java + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 https://git.io/JvXDl + + # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines + # and modify them (or add more) to build your code if your project + # uses a compiled language + + #- run: | + # make bootstrap + # make release + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 diff --git a/.github/workflows/pythonpublish.yml b/.github/workflows/pythonpublish.yml index 87ce4bb7c..75af2084b 100644 --- a/.github/workflows/pythonpublish.yml +++ b/.github/workflows/pythonpublish.yml @@ -10,19 +10,19 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - name: Set up Python - uses: actions/setup-python@v2 - with: - python-version: '3.x' - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install setuptools wheel twine - - name: Build and publish - env: - TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }} - TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} - run: | - python setup.py sdist bdist_wheel - twine upload dist/* + - uses: actions/checkout@v2 + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: '3.x' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install setuptools wheel twine + - name: Build and publish + env: + TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }} + TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} + run: | + python setup.py sdist bdist_wheel + twine upload dist/* diff --git a/qbittorrentapi/auth.py b/qbittorrentapi/auth.py index 8dadd034f..488d2c475 100644 --- a/qbittorrentapi/auth.py +++ b/qbittorrentapi/auth.py @@ -26,13 +26,13 @@ def is_logged_in(self): """Implements :meth:`~AuthAPIMixIn.is_logged_in`""" return self._client.is_logged_in - def log_in(self, username=None, password=None): + def log_in(self, username=None, password=None, **kwargs): """Implements :meth:`~AuthAPIMixIn.auth_log_in`""" - return self._client.auth_log_in(username=username, password=password) + return self._client.auth_log_in(username=username, password=password, **kwargs) - def log_out(self): + def log_out(self, **kwargs): """Implements :meth:`~AuthAPIMixIn.auth_log_out`""" - return self._client.auth_log_out() + return self._client.auth_log_out(**kwargs) class AuthAPIMixIn(Request): @@ -89,21 +89,16 @@ def auth_log_in(self, username=None, password=None, **kwargs): self._password = password or "" self._initialize_context() - self._post( - _name=APINames.Authorization, - _method="login", - data={"username": self.username, "password": self._password}, - **kwargs - ) + creds = {"username": self.username, "password": self._password} + self._post(_name=APINames.Authorization, _method="login", data=creds, **kwargs) if not self.is_logged_in: - logger.debug('Login failed for user "%s"' % self.username) + logger.debug('Login failed for user "%s"', self.username) raise self._suppress_context( LoginFailed('Login authorization failed for user "%s"' % self.username) ) - else: - logger.debug('Login successful for user "%s"' % self.username) - logger.debug("SID: %s" % self._SID) + logger.debug('Login successful for user "%s"', self.username) + logger.debug("SID: %s", self._SID) @property def _SID(self): diff --git a/qbittorrentapi/decorators.py b/qbittorrentapi/decorators.py index a3a431c12..f115021b3 100644 --- a/qbittorrentapi/decorators.py +++ b/qbittorrentapi/decorators.py @@ -85,28 +85,28 @@ def boring_method(): return aliased_class -def login_required(f): +def login_required(func): """ Ensure client is logged in before calling API methods. """ - @wraps(f) + @wraps(func) def wrapper(client, *args, **kwargs): if not client.is_logged_in: logger.debug("Not logged in...attempting login") - client.auth_log_in() + client.auth_log_in(requests_args=client._get_requests_args(**kwargs)) try: - return f(client, *args, **kwargs) + return func(client, *args, **kwargs) except HTTP403Error: logger.debug("Login may have expired...attempting new login") - client.auth_log_in() + client.auth_log_in(requests_args=client._get_requests_args(**kwargs)) - return f(client, *args, **kwargs) + return func(client, *args, **kwargs) return wrapper -def handle_hashes(f): +def handle_hashes(func): """ Normalize torrent hash arguments. @@ -118,13 +118,13 @@ def handle_hashes(f): arguments into either 'torrent_hash' or 'torrent_hashes'. """ - @wraps(f) + @wraps(func) def wrapper(client, *args, **kwargs): if "torrent_hash" not in kwargs and "hash" in kwargs: kwargs["torrent_hash"] = kwargs.pop("hash") elif "torrent_hashes" not in kwargs and "hashes" in kwargs: kwargs["torrent_hashes"] = kwargs.pop("hashes") - return f(client, *args, **kwargs) + return func(client, *args, **kwargs) return wrapper @@ -137,10 +137,10 @@ def response_text(response_class): :return: Text of the response casted to the specified class """ - def _inner(f): - @wraps(f) - def wrapper(obj, *args, **kwargs): - result = f(obj, *args, **kwargs) + def _inner(func): + @wraps(func) + def wrapper(client, *args, **kwargs): + result = func(client, *args, **kwargs) if isinstance(result, response_class): return result try: @@ -155,7 +155,6 @@ def wrapper(obj, *args, **kwargs): def response_json(response_class): - """ Return the JSON in the API response. JSON is parsed as instance of response_class. @@ -163,13 +162,13 @@ def response_json(response_class): :return: JSON as the response class """ - def _inner(f): - @wraps(f) - def wrapper(obj, *args, **kwargs): - simple_response = obj._SIMPLE_RESPONSES or kwargs.pop( + def _inner(func): + @wraps(func) + def wrapper(client, *args, **kwargs): + simple_response = client._SIMPLE_RESPONSES or kwargs.pop( "SIMPLE_RESPONSES", kwargs.pop("SIMPLE_RESPONSE", False) ) - response = f(obj, *args, **kwargs) + response = func(client, *args, **kwargs) try: if isinstance(response, response_class): return response @@ -181,7 +180,7 @@ def wrapper(obj, *args, **kwargs): result = loads(response.text) if simple_response: return result - return response_class(result, obj) + return response_class(result, client) except Exception as e: logger.debug("Exception during response parsing.", exc_info=True) raise APIError("Exception during response parsing. Error: %s" % repr(e)) @@ -228,8 +227,8 @@ def endpoint_introduced(version_introduced, endpoint): :param endpoint: API endpoint (e.g. /torrents/categories) """ - def _inner(f): - @wraps(f) + def _inner(func): + @wraps(func) def wrapper(client, *args, **kwargs): # if the endpoint doesn't exist, return None or log an error / raise an Exception @@ -243,7 +242,7 @@ def wrapper(client, *args, **kwargs): return None # send request to endpoint - return f(client, *args, **kwargs) + return func(client, *args, **kwargs) return wrapper @@ -258,8 +257,8 @@ def version_removed(version_obsoleted, endpoint): :param endpoint: name of the removed endpoint """ - def _inner(f): - @wraps(f) + def _inner(func): + @wraps(func) def wrapper(client, *args, **kwargs): # if the endpoint doesn't exist, return None or log an error / raise an Exception @@ -273,7 +272,7 @@ def wrapper(client, *args, **kwargs): return None # send request to endpoint - return f(client, *args, **kwargs) + return func(client, *args, **kwargs) return wrapper diff --git a/qbittorrentapi/request.py b/qbittorrentapi/request.py index 2e6bb1231..0493e4f65 100644 --- a/qbittorrentapi/request.py +++ b/qbittorrentapi/request.py @@ -1,7 +1,6 @@ -from logging import getLogger from logging import NullHandler +from logging import getLogger from os import environ -from pkg_resources import parse_version from time import sleep try: # python 3 @@ -13,6 +12,7 @@ from urlparse import urljoin from urlparse import urlparse +from pkg_resources import parse_version from requests import exceptions as requests_exceptions from requests import head as requests_head from requests import Session @@ -95,7 +95,9 @@ def _is_version_less_than(cls, ver1, ver2, lteq=True): class Request(HelpersMixIn): - """Facilitates HTTP requests to qBittorrent.""" + """ + Facilitates HTTP requests to qBittorrent's Web API. + """ def __init__(self, host="", port=None, username=None, password=None, **kwargs): self.host = host @@ -119,6 +121,7 @@ def _initialize_context(self): This is necessary on startup or when the auth cookie needs to be replaced...perhaps because it expired, qBittorrent was restarted, significant settings changes, etc. """ + logger.debug("Re-initializing context...") # base path for all API endpoints self._API_BASE_PATH = "api/v2" @@ -143,6 +146,7 @@ def _initialize_context(self): def _initialize_lesser( self, EXTRA_HEADERS=None, + REQUESTS_ARGS=None, VERIFY_WEBUI_CERTIFICATE=True, FORCE_SCHEME_FROM_HOST=False, RAISE_UNIMPLEMENTEDERROR_FOR_UNIMPLEMENTED_API_ENDPOINTS=False, @@ -156,7 +160,8 @@ def _initialize_lesser( """Initialize lessor used configuration""" # Configuration parameters - self._EXTRA_HEADERS = EXTRA_HEADERS if isinstance(EXTRA_HEADERS, dict) else {} + self._EXTRA_HEADERS = EXTRA_HEADERS or {} + self._REQUESTS_ARGS = REQUESTS_ARGS or {} self._VERIFY_WEBUI_CERTIFICATE = bool(VERIFY_WEBUI_CERTIFICATE) self._VERBOSE_RESPONSE_LOGGING = bool(VERBOSE_RESPONSE_LOGGING) self._PRINT_STACK_FOR_EACH_REQUEST = bool(PRINT_STACK_FOR_EACH_REQUEST) @@ -213,11 +218,11 @@ def _post(self, _name=APINames.EMPTY, _method="", **kwargs): http_method="post", api_namespace=_name, api_method=_method, **kwargs ) - def _request_manager(self, _retries=2, _retry_backoff_factor=0.3, **kwargs): + def _request_manager(self, _retries=1, _retry_backoff_factor=0.3, **kwargs): """ Wrapper to manage request retries and severe exceptions. - This should retry at least twice to account for the Web API switching from HTTP to HTTPS. + This should retry at least once to account for the Web API switching from HTTP to HTTPS. During the second attempt, the URL is rebuilt using HTTP or HTTPS as appropriate. """ @@ -250,9 +255,9 @@ def retry_backoff(retry_count): # then will sleep for 0s then .3s, then .6s, etc. between retries. backoff_time = _retry_backoff_factor * (2 ** ((retry_count + 1) - 1)) sleep(backoff_time if backoff_time <= 10 else 10) - logger.debug("Retry attempt %d", (retry_count + 1)) + logger.debug("Retry attempt %d", retry_count + 1) - max_retries = _retries if _retries > 1 else 2 + max_retries = _retries if _retries >= 1 else 1 for retry in range(0, (max_retries + 1)): try: return self._request(**kwargs) @@ -280,18 +285,19 @@ def _request(self, http_method, api_namespace, api_method, **kwargs): :param kwargs: see _normalize_requests_params for additional support :return: Requests response """ - url = self._build_url(api_namespace=api_namespace, api_method=api_method) - kwargs = self._trim_known_kwargs(kwargs=kwargs) - params = self._normalize_requests_params(http_method=http_method, **kwargs) + api_args, requests_args = self._normalize_args(http_method, **kwargs) + url = self._build_url(api_namespace, api_method, requests_args=requests_args) - response = self._session.request(method=http_method, url=url, **params) + http_args = api_args.copy() + http_args.update(requests_args) + response = self._session.request(http_method, url, **http_args) - self.verbose_logging(http_method=http_method, response=response, url=url) - self.handle_error_responses(params=params, response=response) + self._verbose_logging(http_method, url, http_args, response) + self._handle_error_responses(api_args, response) return response @staticmethod - def _trim_known_kwargs(kwargs): + def _trim_known_kwargs(**kwargs): """ Since any extra keyword arguments from the user are automatically included in the request to qBittorrent, this removes any "known" @@ -306,7 +312,17 @@ def _trim_known_kwargs(kwargs): kwargs.pop("SIMPLE_RESPONSE", None) return kwargs - def _build_url(self, api_namespace, api_method): + @staticmethod + def _get_requests_args(**kwargs): + """ + Return any user-supplied arguments for Requests. + """ + args = kwargs.get("requests_args", kwargs.get("requests_params", {})) or {} + if "headers" in kwargs: + args.setdefault("headers", {}).update(kwargs["headers"] or {}) + return args + + def _build_url(self, api_namespace, api_method, requests_args): """ Create a fully qualified URL for the API endpoint. @@ -319,6 +335,7 @@ def _build_url(self, api_namespace, api_method): host=self.host, port=self.port, force_user_scheme=self._FORCE_SCHEME_FROM_HOST, + requests_args=requests_args, ) return self._build_url_path( base_url=self._API_BASE_URL, @@ -327,8 +344,14 @@ def _build_url(self, api_namespace, api_method): api_method=api_method, ) - @staticmethod - def _build_base_url(base_url=None, host="", port=None, force_user_scheme=False): + def _build_base_url( + self, + base_url=None, + host="", + port=None, + force_user_scheme=False, + requests_args=None, + ): """ Determine the Base URL for the Web API endpoints. @@ -359,34 +382,46 @@ def _build_base_url(base_url=None, host="", port=None, force_user_scheme=False): logger.debug("Parsed user URL: %r", base_url) # default to HTTP if user didn't specify user_scheme = base_url.scheme - base_url = base_url._replace(scheme="http") if not user_scheme else base_url - alt_scheme = "https" if base_url.scheme == "http" else "http" + default_scheme = user_scheme or "http" + alt_scheme = "https" if default_scheme == "http" else "http" # add port number if host doesn't contain one - if port is not None and not isinstance(base_url.port, int): - base_url = base_url._replace(netloc="%s:%s" % (base_url.netloc, port)) + if port is not None and not base_url.port: + host = "".join((base_url.netloc, ":", str(port))) + base_url = base_url._replace(netloc=host) # detect whether Web API is configured for HTTP or HTTPS if not (user_scheme and force_user_scheme): logger.debug("Detecting scheme for URL...") - try: - # skip verification here...if there's a problem, we'll catch it during the actual API call - r = requests_head(base_url.geturl(), allow_redirects=True, verify=False) - # if WebUI eventually supports sending a redirect from HTTP to HTTPS then - # Requests will automatically provide a URL using HTTPS. - # For instance, the URL returned below will use the HTTPS scheme. - # >>> requests.head('http://grc.com', allow_redirects=True).url - scheme = urlparse(r.url).scheme - except requests_exceptions.RequestException: - # assume alternative scheme will work...we'll fail later if neither are working - scheme = alt_scheme + prefer_https = False + for scheme in (default_scheme, alt_scheme): + try: + base_url = base_url._replace(scheme=scheme) + r = self._session.request( + "head", base_url.geturl(), **requests_args + ) + scheme_to_use = urlparse(r.url).scheme + break + except requests_exceptions.SSLError: + # an SSLError means that qBittorrent is likely listening on HTTPS + # but the TLS connection is not trusted...so, if the attempt to + # connect on HTTP also fails, this will tell us to switch back to HTTPS + if base_url.scheme.lower() == "https": + logger.debug( + "Encountered SSLError: will prefer HTTPS if HTTP fails" + ) + prefer_https = True + except requests_exceptions.RequestException: + logger.debug("Failed connection attempt with %s", scheme.upper()) + else: + scheme_to_use = "https" if prefer_https else "http" # use detected scheme - logger.debug("Using %s scheme", scheme.upper()) - base_url = base_url._replace(scheme=scheme) - if user_scheme and user_scheme != scheme: + logger.debug("Using %s scheme", scheme_to_use.upper()) + base_url = base_url._replace(scheme=scheme_to_use) + if user_scheme and user_scheme != scheme_to_use: logger.warning( "Using '%s' instead of requested '%s' to communicate with qBittorrent", - scheme, + scheme_to_use, user_scheme, ) @@ -396,6 +431,9 @@ def _build_base_url(base_url=None, host="", port=None, force_user_scheme=False): base_url = base_url + "/" logger.debug("Base URL: %s", base_url) + # force a new session to be created now that the URL is known + self._requests_session = None + return base_url @staticmethod @@ -430,10 +468,23 @@ def _session(self): :return: Requests Session object """ + + class QbittorrentSession(Session): + """ + Wrapper to augment Requests Session. + Requests doesn't allow Session to default certain configuration + globally. This gets around that by setting defaults for each request. + """ + + def request(self, *args, **kwargs): + kwargs.setdefault("timeout", 30) + kwargs.setdefault("allow_redirects", True) + return super(QbittorrentSession, self).request(*args, **kwargs) + if self._requests_session: return self._requests_session - self._requests_session = Session() + self._requests_session = QbittorrentSession() # default headers to prevent qBittorrent throwing any alarms self._requests_session.headers.update( @@ -458,28 +509,29 @@ def _session(self): # at any rate, the retries count in request_manager should always be # at least 2 to accommodate significant settings changes in qBittorrent # such as enabling HTTPs in Web UI settings. - retries = 1 - retry = Retry( - total=retries, - read=retries, - connect=retries, - status_forcelist={500, 502, 504}, - raise_on_status=False, + adapter = HTTPAdapter( + max_retries=Retry( + total=1, + read=1, + connect=1, + status_forcelist={500, 502, 504}, + raise_on_status=False, + ) ) - adapter = HTTPAdapter(max_retries=retry) self._requests_session.mount("http://", adapter) self._requests_session.mount("https://", adapter) return self._requests_session - @staticmethod - def _normalize_requests_params( + def _normalize_args( + self, http_method, headers=None, data=None, params=None, files=None, requests_params=None, + requests_args=None, **kwargs ): """ @@ -489,24 +541,32 @@ def _normalize_requests_params( :param data: key/value pairs to send as body :param params: key/value pairs to send as query parameters :param files: key/value pairs to include as multipart POST requests - :param requests_params: keyword arguments for call to Requests + :param requests_params: original name for requests_args + :param requests_args: keyword arguments for call to Requests :return: dictionary of parameters for Requests call """ + kwargs = self._trim_known_kwargs(**kwargs) + # these are completely user defined and intended to allow users # of this client to control the behavior of Requests - requests_params = requests_params or {} + override_requests_args = self._get_requests_args( + headers=headers, + requests_params=requests_params, + requests_args=requests_args, + ) # these are expected to be populated by this client as necessary for qBittorrent data = data or {} params = params or {} files = files or {} - # these are user-defined headers to include with the request - headers = headers or {} # send Content-Length as 0 for empty POSTs...Requests will not send Content-Length - # if data is empty but qBittorrent may complain otherwise - if http_method == "post" and not any(filter(None, data.values())): - headers["Content-Length"] = "0" + # if data is empty but qBittorrent will complain otherwise + is_data_content = any(filter(lambda x: x is None, data.values())) + if http_method.lower() == "post" and not is_data_content: + override_requests_args.setdefault("headers", {}).update( + {"Content-Length": "0"} + ) # any other keyword arguments are sent to qBittorrent as part of the request. # These are user-defined since this Client will put everything in data/params/files @@ -517,17 +577,21 @@ def _normalize_requests_params( if http_method == "post": data.update(kwargs) - d = dict(data=data, params=params, files=files, headers=headers) - d.update(requests_params) - return d + # arguments specified during Client construction + requests_args = self._REQUESTS_ARGS.copy() + # incorporate arguments specified for this specific API call + requests_args.update(override_requests_args) + # qbittorrent api call arguments + api_args = dict(data=data, params=params, files=files) + return api_args, requests_args @staticmethod - def handle_error_responses(params, response): + def _handle_error_responses(args, response): """Raise proper exception if qBittorrent returns Error HTTP Status.""" if response.status_code < 400: # short circuit for non-error statuses return - elif response.status_code == 400: + if response.status_code == 400: # Returned for malformed requests such as missing or invalid parameters. # # If an error_message isn't returned, qBittorrent didn't receive all required parameters. @@ -537,53 +601,49 @@ def handle_error_responses(params, response): raise MissingRequiredParameters400Error() raise InvalidRequest400Error(response.text) - elif response.status_code == 401: + if response.status_code == 401: # Primarily reserved for XSS and host header issues. Is also raise Unauthorized401Error(response.text) - elif response.status_code == 403: + if response.status_code == 403: # Not logged in or calling an API method that isn't public # APIErrorType::AccessDenied raise Forbidden403Error(response.text) - elif response.status_code == 404: + if response.status_code == 404: # API method doesn't exist or more likely, torrent not found # APIErrorType::NotFound error_message = response.text if error_message in ("", "Not Found"): error_torrent_hash = "" - if params["data"]: - error_torrent_hash = params["data"].get("hash", error_torrent_hash) - error_torrent_hash = params["data"].get( - "hashes", error_torrent_hash - ) - if params and error_torrent_hash == "": - error_torrent_hash = params["params"].get( - "hash", error_torrent_hash - ) - error_torrent_hash = params["params"].get( + if args["data"]: + error_torrent_hash = args["data"].get("hash", error_torrent_hash) + error_torrent_hash = args["data"].get("hashes", error_torrent_hash) + if args and error_torrent_hash == "": + error_torrent_hash = args["params"].get("hash", error_torrent_hash) + error_torrent_hash = args["params"].get( "hashes", error_torrent_hash ) if error_torrent_hash: error_message = "Torrent hash(es): %s" % error_torrent_hash raise NotFound404Error(error_message) - elif response.status_code == 409: + if response.status_code == 409: # APIErrorType::Conflict raise Conflict409Error(response.text) - elif response.status_code == 415: + if response.status_code == 415: # APIErrorType::BadData raise UnsupportedMediaType415Error(response.text) - elif response.status_code >= 500: + if response.status_code >= 500: raise InternalServerError500Error(response.text) - elif response.status_code >= 400: + if response.status_code >= 400: # Unaccounted for errors from API raise HTTPError(response.text) - def verbose_logging(self, http_method, response, url): + def _verbose_logging(self, http_method, url, http_args, response): """Log verbose information about request. Can be useful during development.""" if self._VERBOSE_RESPONSE_LOGGING: resp_logger = logger.debug @@ -592,8 +652,9 @@ def verbose_logging(self, http_method, response, url): # log as much as possible in a error condition max_text_length_to_log = 10000 - resp_logger("Request URL: (%s) %s" % (http_method.upper(), response.url)) - resp_logger("Request Headers: %s" % response.request.headers) + resp_logger("Request URL: (%s) %s", http_method.upper(), response.url) + resp_logger("Request HTTP Args: %s", http_args) + resp_logger("Request Headers: %s", response.request.headers) if ( str(response.request.body) not in ("None", "") and "auth/login" not in url @@ -604,15 +665,13 @@ def verbose_logging(self, http_method, response, url): else len(response.request.body) ) resp_logger( - "Request body: %s%s" - % ( - response.request.body[:body_len], - "..." if body_len >= 200 else "", - ) + "Request body: %s%s", + response.request.body[:body_len], + "..." if body_len >= 200 else "", ) resp_logger( - "Response status: %s (%s)" % (response.status_code, response.reason) + "Response status: %s (%s)", response.status_code, response.reason ) if response.text: text_len = ( @@ -621,11 +680,9 @@ def verbose_logging(self, http_method, response, url): else len(response.text) ) resp_logger( - "Response text: %s%s" - % ( - response.text[:text_len], - "..." if text_len >= 80 else "", - ) + "Response text: %s%s", + response.text[:text_len], + "..." if text_len >= 80 else "", ) if self._PRINT_STACK_FOR_EACH_REQUEST: from traceback import print_stack diff --git a/qbittorrentapi/rss.py b/qbittorrentapi/rss.py index 22ec0f298..97edeb4e1 100644 --- a/qbittorrentapi/rss.py +++ b/qbittorrentapi/rss.py @@ -150,7 +150,7 @@ def rss_add_folder(self, folder_path=None, **kwargs): :raises Conflict409Error: - :param folder_path: path to new folder (e.g. Linux\ISOs) + :param folder_path: path to new folder (e.g. Linux\\ISOs) :return: None """ data = {"path": folder_path} @@ -165,7 +165,7 @@ def rss_add_feed(self, url=None, item_path=None, **kwargs): :raises Conflict409Error: :param url: URL of RSS feed (e.g http://thepiratebay.org/rss/top100/200) - :param item_path: Name and/or path for new feed (e.g. Folder\Subfolder\FeedName) + :param item_path: Name and/or path for new feed (e.g. Folder\\Subfolder\\FeedName) :return: None """ data = {"url": url, "path": item_path} @@ -181,7 +181,7 @@ def rss_remove_item(self, item_path=None, **kwargs): :raises Conflict409Error: - :param item_path: path to item to be removed (e.g. Folder\Subfolder\ItemName) + :param item_path: path to item to be removed (e.g. Folder\\Subfolder\\ItemName) :return: None """ data = {"path": item_path} @@ -195,8 +195,8 @@ def rss_move_item(self, orig_item_path=None, new_item_path=None, **kwargs): :raises Conflict409Error: - :param orig_item_path: path to item to be removed (e.g. Folder\Subfolder\ItemName) - :param new_item_path: path to item to be removed (e.g. Folder\Subfolder\ItemName) + :param orig_item_path: path to item to be removed (e.g. Folder\\Subfolder\\ItemName) + :param new_item_path: path to item to be removed (e.g. Folder\\Subfolder\\ItemName) :return: None """ data = {"itemPath": orig_item_path, "destPath": new_item_path} @@ -221,7 +221,7 @@ def rss_refresh_item(self, item_path=None, **kwargs): """ Trigger a refresh for a RSS item (alias: rss_refreshItem) - :param item_path: path to item to be refreshed (e.g. Folder\Subfolder\ItemName) + :param item_path: path to item to be refreshed (e.g. Folder\\Subfolder\\ItemName) :return: None """ # HACK: v4.1.7 and v4.1.8 both use api v2.2; however, refreshItem was introduced in v4.1.8 @@ -238,7 +238,7 @@ def rss_mark_as_read(self, item_path=None, article_id=None, **kwargs): :raises NotFound404Error: - :param item_path: path to item to be refreshed (e.g. Folder\Subfolder\ItemName) + :param item_path: path to item to be refreshed (e.g. Folder\\Subfolder\\ItemName) :param article_id: article ID from rss_items() :return: None """ diff --git a/requirements-dev.txt b/requirements-dev.txt index 8a1f1be0c..3bdb6ecd4 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -6,4 +6,4 @@ coveralls pytest pytest-cov sphinx -sphinx_glpi_theme \ No newline at end of file +sphinx_glpi_theme diff --git a/tests/test_request.py b/tests/test_request.py index 070dee44c..eb2aae39c 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -147,11 +147,58 @@ def test_port(api_version): assert client.app.web_api_version == api_version -def test_simple_response(client): +def test_simple_response(client, orig_torrent): torrent = client.torrents_info()[0] assert isinstance(torrent, TorrentDictionary) torrent = client.torrents_info(SIMPLE_RESPONSE=True)[0] assert isinstance(torrent, dict) + torrent = client.torrents_info(SIMPLE_RESPONSES=True)[0] + assert isinstance(torrent, dict) + torrent = client.torrents_info(SIMPLE_RESPONSE=False)[0] + assert isinstance(torrent, TorrentDictionary) + torrent = client.torrents_info(SIMPLE_RESPONSES=False)[0] + assert isinstance(torrent, TorrentDictionary) + client = Client(VERIFY_WEBUI_CERTIFICATE=False, SIMPLE_RESPONSES=True) + torrent = client.torrents_info()[0] + assert isinstance(torrent, dict) + client = Client(VERIFY_WEBUI_CERTIFICATE=False, SIMPLE_RESPONSES=False) + torrent = client.torrents_info()[0] + assert isinstance(torrent, TorrentDictionary) + + +def test_request_extra_headers(): + client = Client( + VERIFY_WEBUI_CERTIFICATE=False, EXTRA_HEADERS={"X-MY-HEADER": "asdf"} + ) + client.auth.log_in() + + r = client._get(_name="app", _method="version") + assert r.request.headers["X-MY-HEADER"] == "asdf" + + r = client._get(_name="app", _method="version", headers={"X-MY-HEADER-TWO": "zxcv"}) + assert r.request.headers["X-MY-HEADER"] == "asdf" + assert r.request.headers["X-MY-HEADER-TWO"] == "zxcv" + + r = client._get( + _name="app", + _method="version", + headers={"X-MY-HEADER-TWO": "zxcv"}, + requests_args={"headers": {"X-MY-HEADER-THREE": "tyui"}}, + ) + assert r.request.headers["X-MY-HEADER"] == "asdf" + assert r.request.headers["X-MY-HEADER-TWO"] == "zxcv" + assert r.request.headers["X-MY-HEADER-THREE"] == "tyui" + + r = client._get( + _name="app", + _method="version", + headers={"X-MY-HEADER": "zxcv"}, + requests_args={"headers": {"X-MY-HEADER": "tyui"}}, + ) + assert r.request.headers["X-MY-HEADER"] == "zxcv" + + r = client._get(_name="app", _method="version", headers={"X-MY-HEADER": "zxcv"}) + assert r.request.headers["X-MY-HEADER"] == "zxcv" def test_request_extra_params(client, orig_torrent_hash): @@ -257,7 +304,7 @@ def test_http404(client, params): response = MockResponse(status_code=404, text="") with pytest.raises(HTTPError): p = dict(data={}, params=params) - Request.handle_error_responses(params=p, response=response) + Request._handle_error_responses(args=p, response=response) def test_http409(client, app_version): @@ -276,7 +323,7 @@ def test_http500(status_code): response = MockResponse(status_code=status_code, text="asdf") with pytest.raises(InternalServerError500Error): p = dict(data={}, params={}) - Request.handle_error_responses(params=p, response=response) + Request._handle_error_responses(args=p, response=response) @pytest.mark.parametrize("status_code", (402, 406)) @@ -284,7 +331,7 @@ def test_http_error(status_code): response = MockResponse(status_code=status_code, text="asdf") with pytest.raises(HTTPError): p = dict(data={}, params={}) - Request.handle_error_responses(params=p, response=response) + Request._handle_error_responses(args=p, response=response) def test_verbose_logging(caplog):