Repository: snowflakedb/snowflake-connector-python

Issue:
SNOW-752200: Invalide None values in fetchmany and fetchall introduced in 3.0.1
1. What version of Python are you using?

```
> python --version --version
Python 3.11.2 (main, Feb 13 2023, 17:10:22) [Clang 14.0.0 (clang-1400.0.29.202)]
```

2. What operating system and processor architecture are you using?

```
> python -c 'import platform; print(platform.platform())'
macOS-13.2.1-arm64-arm-64bit
```

3. What are the component versions in the environment (`pip freeze`)?

```
> python -m pip freeze | grep -E 'mypy=|snowflake'
mypy==1.0.1
snowflake-connector-python==3.0.1
```


4. What did you do?

Ran mypy against our code using snowflake-connector-python

5. What did you expect to see?

   No error

6. Can you set logging to DEBUG and collect the logs?

N/A


In the 3.0.1 release the `fetchmany` and `fetchall` method got their return types updated to `list[dict | tuple | None]`. This is actually incorrect since they both contain logic to exclude row values that are `None`. Our own code is expecting lists of dicts or tuples explicitly and the possibility of getting None row is breaking mypy.

The return types should be `list[dict | tuple]`.

Discussion:
codecov-commenter: # [Codecov](https://codecov.io/gh/snowflakedb/snowflake-connector-python/pull/1465?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing) Report
> Merging [#1465](https://codecov.io/gh/snowflakedb/snowflake-connector-python/pull/1465?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing) (f153641) into [main](https://codecov.io/gh/snowflakedb/snowflake-connector-python/commit/ec95c563ded4694f69e8bde4eb2f010f92681e58?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing) (ec95c56) will **decrease** coverage by `0.11%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main    #1465      +/-   ##
==========================================
- Coverage   81.91%   81.81%   -0.11%     
==========================================
  Files          60       60              
  Lines        8599     8599              
  Branches     1271     1271              
==========================================
- Hits         7044     7035       -9     
- Misses       1225     1232       +7     
- Partials      330      332       +2     
```


| [Impacted Files](https://codecov.io/gh/snowflakedb/snowflake-connector-python/pull/1465?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing) | Coverage Δ | |
|---|---|---|
| [src/snowflake/connector/cursor.py](https://codecov.io/gh/snowflakedb/snowflake-connector-python/pull/1465?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing#diff-c3JjL3Nub3dmbGFrZS9jb25uZWN0b3IvY3Vyc29yLnB5) | `94.66% <100.00%> (ø)` | |
| [src/snowflake/connector/ocsp\_snowflake.py](https://codecov.io/gh/snowflakedb/snowflake-connector-python/pull/1465?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing#diff-c3JjL3Nub3dmbGFrZS9jb25uZWN0b3Ivb2NzcF9zbm93Zmxha2UucHk=) | `76.65% <0.00%> (-1.08%)` | :arrow_down: |
| [src/snowflake/connector/cache.py](https://codecov.io/gh/snowflakedb/snowflake-connector-python/pull/1465?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing#diff-c3JjL3Nub3dmbGFrZS9jb25uZWN0b3IvY2FjaGUucHk=) | `87.62% <0.00%> (-0.34%)` | :arrow_down: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Snowflake+Computing)


Patch:
diff --git a/src/snowflake/connector/cursor.py b/src/snowflake/connector/cursor.py
index 16308163..92a574a1 100644
--- a/src/snowflake/connector/cursor.py
+++ b/src/snowflake/connector/cursor.py
@@ -1205,7 +1205,7 @@ class SnowflakeCursor:
         except StopIteration:
             return None
 
-    def fetchmany(self, size: int | None = None) -> list[dict | tuple | None]:
+    def fetchmany(self, size: int | None = None) -> list[tuple] | list[dict]:
         """Fetches the number of specified rows."""
         if size is None:
             size = self.arraysize
@@ -1231,7 +1231,7 @@ class SnowflakeCursor:
 
         return ret
 
-    def fetchall(self) -> list[dict | tuple | None]:
+    def fetchall(self) -> list[tuple] | list[dict]:
         """Fetches all of the results."""
         ret = []
         while True:
diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py
index e6814579..6720d237 100644
--- a/src/snowflake/connector/network.py
+++ b/src/snowflake/connector/network.py
@@ -20,6 +20,7 @@ from typing import TYPE_CHECKING, Any
 
 import OpenSSL.SSL
 
+from snowflake.connector.secret_detector import SecretDetector
 from snowflake.connector.vendored.requests.models import PreparedRequest
 from snowflake.connector.vendored.urllib3.connectionpool import (
     HTTPConnectionPool,
@@ -980,14 +981,9 @@ class SnowflakeRestful:
     def _handle_unknown_error(self, method, full_url, headers, data, conn) -> None:
         """Handles unknown errors."""
         if data:
-            try:
-                decoded_data = json.loads(data)
-                if decoded_data.get("data") and decoded_data["data"].get("PASSWORD"):
-                    # masking the password
-                    decoded_data["data"]["PASSWORD"] = "********"
-                    data = json.dumps(decoded_data)
-            except Exception:
-                logger.info("data is not JSON")
+            _, masked_data, err_str = SecretDetector.mask_secrets(data)
+            if err_str is None:
+                data = masked_data
         logger.error(
             f"Failed to get the response. Hanging? "
             f"method: {method}, url: {full_url}, headers:{headers}, "


Test patch:
diff --git a/test/unit/test_retry_network.py b/test/unit/test_retry_network.py
index ba983bdf..7f7e69b5 100644
--- a/test/unit/test_retry_network.py
+++ b/test/unit/test_retry_network.py
@@ -6,6 +6,7 @@
 from __future__ import annotations
 
 import errno
+import logging
 import os
 import time
 from unittest.mock import MagicMock, Mock, PropertyMock
@@ -217,3 +218,40 @@ def test_fetch():
     assert ret == {}
     assert cnt.c == 1  # failed on first call - did not retry
     assert rest._connection.errorhandler.called  # error
+
+
+def test_secret_masking(caplog):
+    connection = MagicMock()
+    connection.errorhandler = Mock(return_value=None)
+
+    rest = SnowflakeRestful(
+        host="testaccount.snowflakecomputing.com", port=443, connection=connection
+    )
+
+    data = (
+        '{"code": 12345,'
+        ' "data": {"TOKEN": "_Y1ZNETTn5/qfUWj3Jedb", "PASSWORD": "dummy_pass"}'
+        "}"
+    )
+    default_parameters = {
+        "method": "POST",
+        "full_url": "https://testaccount.snowflakecomputing.com/",
+        "headers": {},
+        "data": data,
+    }
+
+    class NotRetryableException(Exception):
+        pass
+
+    def fake_request_exec(**kwargs):
+        return None
+
+    # inject a fake method
+    rest._request_exec = fake_request_exec
+
+    # first two attempts will fail but third will success
+    with caplog.at_level(logging.ERROR):
+        ret = rest.fetch(timeout=10, **default_parameters)
+    assert '"TOKEN": "****' in caplog.text
+    assert '"PASSWORD": "****' in caplog.text
+    assert ret == {}