IBM-Cloud / sql-query-clients

Client samples for IBM Cloud SQL Query service
Apache License 2.0
12 stars 24 forks source link

Bug: Mismatched comparison when failing login #140

Closed NielsKorschinsky closed 1 year ago

NielsKorschinsky commented 1 year ago

Describe the bug

When failing a login and the max re-tries are reached, the code does not output its desired AttributeError("Login fails: credential cannot be validated - check either (1) the key or (2) if IBM cloud service is available"). Instead, it skips this check and fails when trying to access a None-Object.

This results in a cryptic error message:

lib/python3.9/site-packages/ibmcloudsql/utilities.py", line 308, in logon
    {"authorization": "Bearer {}".format(ro_credentials.token)}
AttributeError: 'NoneType' object has no attribute 'token'

To Reproduce

from ibmcloudsql import SQLQuery

cern_token="crn:v1:CorrectCERNToken::"
api_token="TokenWithNoPermissions"

sql_client = SQLQuery(api_token, cern_token)
sql_client.logon()

Expected behavior

A error, describing that the login failed.

Additional context

I've already developed a fix, including it in comments.

NielsKorschinsky commented 1 year ago

File: .venv/lib/python3.9/site-packages/ibmcloudsql/utilities.py

Line: 269 to 301

Error:

The program starts with count = 0 and self._iam_max_tries = 1. Then, after failing in line 279, it raises an exception, catched in line 281. There, the count is increased by 1: count +=1 Now, it is checked whether the max re-run count is reached. if so, an exception is thrown, else continued. The check is following: if count > self._iam_max_tries:

However, this should not be a > but a >=, as 1>1 equals false, not triggering the exception.

Due to the non-abort, on the next loop check: while count < self._iam_max_tries it evaluates to false: 1<1.

By changing the first evaluation to if count >= self._iam_max_tries:, the exception will correctly be raised. The same applies to line 291.

Complete code:

        while not complete and count < self._iam_max_tries:
            try:
                # set this to ensure a new token is retrieved
                if not self.token:
                    boto3_session.get_credentials().token_manager._expiry_time = (
                        boto3_session.get_credentials().token_manager._time_fetcher()
                    )
                ro_credentials = (
                    boto3_session.get_credentials().get_frozen_credentials()
                )
                self.current_token = ro_credentials.token
                complete = True
            except ibm_botocore.exceptions.CredentialRetrievalError:
                count += 1
                if count >= self._iam_max_tries:
                    msg = (
                        "Login fails: credential cannot be validated"
                        "- check either (1) the key or (2) if IBM  cloud service is available"
                    )
                    raise AttributeError(msg)
            except requests.exceptions.ReadTimeout:
                count += 1
                if count >= self._iam_max_tries:
                    msg = (
                        "Increase iam_max_tries (current set to {}),"
                        " or relaunch again as no response from IAM"
                    ).format(self._iam_max_tries)
                    raise AttributeError(msg)
            except Exception as e:
                raise e
            finally:
                time.sleep(retry_delay)
                retry_delay = min(retry_delay + 1, 10)
torsstei commented 1 year ago

Thanks for the detailed explanation and fix. Released as v 0.5.12