chore: Replace print statements with logger by iron-prog · Pull Request #2452 · hiero-ledger/hiero-sdk-python

iron-prog · 2026-07-18T15:22:01Z

Description:
Replace print() and traceback.print_exc() with structured logging across query and network modules.

Add module-level loggers to query and network modules
Replace print()/traceback.print_exc() with logger.error(..., exc_info=True) or logger.warning(...)
Remove unused traceback imports
Add regression tests to verify no print() calls remain, logs use the correct module logger, and tracebacks are preserved

Related issue(s):
Fixes #2403

Notes for reviewer:

No public API changes; internal logging only
Docstring print(...) examples were intentionally left unchanged
TransactionGetReceiptQuery uses a different mock target because it builds its header inline

Checklist

Documented
Tested

coderabbitai · 2026-07-18T15:24:54Z

Walkthrough

Query request error paths and mirror-node discovery now use module-level logging instead of direct console output, while preserving re-raises and fallback results. New unit tests verify logger names, levels, exception information, and removal of direct traceback printing.

Changes

Structured error logging

Layer / File(s)	Summary
Query request error logging `src/hiero_sdk_python/account/`, `src/hiero_sdk_python/contract/`, `src/hiero_sdk_python/file/`, `src/hiero_sdk_python/query/`, `src/hiero_sdk_python/schedule/*`	Query `_make_request` handlers use module loggers with `exc_info=True` instead of `print` and `traceback.print_exc()`, then re-raise exceptions.
Mirror-node diagnostic logging `src/hiero_sdk_python/client/network.py`	Mirror-node URL, request, and parsing diagnostics use warning and error logger calls while returning the existing fallback values.
Logging behavior validation `tests/unit/query_error_logging_test.py`	Parameterized tests verify query logging, absence of direct traceback output, and mirror-node warning/error records.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: replacing print-based output with logging.
Description check	✅ Passed	The description matches the changeset and accurately describes the logging updates and tests.
Linked Issues check	✅ Passed	The PR addresses `#2403` by removing direct print/traceback output from the listed query and network paths while preserving behavior.
Out of Scope Changes check	✅ Passed	The added tests and logging-only code changes stay within the stated issue scope, with no obvious unrelated changes.
Docstring Coverage	✅ Passed	Docstring coverage is 88.46% which is sufficient. The required threshold is 80.00%.

📋 Issue Planner

Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).

View plan for ticket: #2403

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

manishdait

@iron-prog, thanks for the PR, just a small change request

manishdait · 2026-07-22T08:39:35Z

-            print(f"Error fetching nodes from mirror node API: {e}")
+            logger.error("Error fetching nodes from mirror node API: %s", e)
+            return []
+        except (ValueError, KeyError, TypeError, IndexError, AttributeError) as e:


I think we can just fall back to a generic Exception here instead of listing all the possible exception types

This problem comes from NodeAddress._from_dict, where missing keys pass None thus you get the variety of type erorrs/attribute errors/etc
So i think we should look at is improve none handling, which maybe should be a different PR what do you think @manishdait ?

exploreriii · 2026-07-22T12:05:36Z

-            print(f"Error fetching nodes from mirror node API: {e}")
+            logger.error("Error fetching nodes from mirror node API: %s", e)
+            return []
+        except (ValueError, KeyError, TypeError, IndexError, AttributeError) as e:


This problem comes from NodeAddress._from_dict, where missing keys pass None thus you get the variety of type erorrs/attribute errors/etc
So i think we should look at is improve none handling, which maybe should be a different PR what do you think @manishdait ?

Signed-off-by: iron-prog <dt915725@gmail.com>

- Replace remaining logging.error(...) (root logger) with logger.error() - Add exc_info=True to file_contents_query.py for traceback parity - Strengthen tests: seed valid IDs, assert emitting logger identity (record.name == module), assert traceback is attached (record.exc_info) Signed-off-by: iron-prog <dt915725@gmail.com>

Signed-off-by: iron-prog <dt915725@gmail.com>

coderabbitai

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/hiero_sdk_python/query/token_info_query.py (1)
69-70: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore required token-ID validation.

if self.token_id now allows None, so _make_request() silently builds a request with an empty/default token instead of raising the documented ValueError. Restore the guard and serialize only after validation.
Suggested fix
         try:
+            if self.token_id is None:
+                raise ValueError("Token ID must be set before making the request.")
             query_header = self._make_request_header()

             token_info_query = token_get_info_pb2.TokenGetInfoQuery()
             token_info_query.header.CopyFrom(query_header)
-            if self.token_id:
-                token_info_query.token.CopyFrom(self.token_id._to_proto())
+            token_info_query.token.CopyFrom(self.token_id._to_proto())
Add a regression test for TokenInfoQuery()._make_request() without a token ID.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: be976864-47ab-41c2-be81-e9bcded72eb1

📥 Commits

Reviewing files that changed from the base of the PR and between 6afbe47 and 24472d2.

📒 Files selected for processing (15)

src/hiero_sdk_python/account/account_records_query.py
src/hiero_sdk_python/client/network.py
src/hiero_sdk_python/contract/contract_bytecode_query.py
src/hiero_sdk_python/contract/contract_call_query.py
src/hiero_sdk_python/contract/contract_info_query.py
src/hiero_sdk_python/file/file_contents_query.py
src/hiero_sdk_python/file/file_info_query.py
src/hiero_sdk_python/query/account_balance_query.py
src/hiero_sdk_python/query/account_info_query.py
src/hiero_sdk_python/query/token_info_query.py
src/hiero_sdk_python/query/token_nft_info_query.py
src/hiero_sdk_python/query/topic_info_query.py
src/hiero_sdk_python/query/transaction_get_receipt_query.py
src/hiero_sdk_python/schedule/schedule_info_query.py
tests/unit/query_error_logging_test.py

coderabbitai · 2026-07-22T13:47:24Z

        except requests.RequestException as e:
-            print(f"Error fetching nodes from mirror node API: {e}")
+            logger.error("Error fetching nodes from mirror node API: %s", e)
+            return []


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

MINOR — Preserve traceback context for mirror-node request failures.

This branch logs only str(e), so the structured record has no exc_info, unlike the parsing branch and query handlers. Add exc_info=True (or use logger.exception) and assert traceback retention in the request-failure test.

coderabbitai · 2026-07-22T13:47:24Z

+    source = inspect.getsource(Network._fetch_nodes_from_mirror_node)
+    assert "print(" not in source


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert that network code does not call traceback.print_exc().

These checks only search for "print("; that does not match traceback.print_exc(), so direct traceback output could be reintroduced while the tests remain green.

Suggested assertion

assert "print(" not in source + assert "traceback.print_exc()" not in source, ( + "Network._fetch_nodes_from_mirror_node still calls traceback.print_exc()" + )

Also applies to: 144-145, 173-174

coderabbitai · 2026-07-22T13:47:24Z

+    assert result == []
+    matching = [
+        record
+        for record in caplog.records
+        if record.levelno == logging.WARNING and "No known mirror node URL" in record.message
+    ]
+    assert matching
+    assert matching[0].name == Network.__module__


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add diagnostic messages to mirror-node assertions.

The new tests use bare assertions for results, matching log records, and logger names, making failures ambiguous. Add descriptive messages to each assertion.

As per path instructions, unit tests must provide useful error messages when they fail.

Also applies to: 161-168, 195-202

Source: Path instructions

coderabbitai · 2026-07-22T13:47:24Z

+        if record.levelno == logging.ERROR and "Error parsing mirror node API response" in record.message
+    ]
+    assert matching
+    assert matching[0].name == Network.__module__


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert traceback retention for malformed responses.

This test exercises the parsing branch that uses exc_info=True, but it only checks the log message and logger name. Add an assertion that matching[0].exc_info is not None so traceback loss is detected.

iron-prog requested review from a team as code owners July 18, 2026 15:22

iron-prog requested review from MonaaEid and Mounil2005 July 18, 2026 15:22

github-actions Bot assigned MonaaEid and Mounil2005 Jul 18, 2026

iron-prog force-pushed the fix-2403-print-to-logger-v2 branch from 6afbe47 to 9aba0b3 Compare July 18, 2026 15:24

github-actions Bot added open to community review PR is open for community review and feedback queue:junior-committer PR awaiting initial quality review labels Jul 18, 2026

Mounil2005 requested changes Jul 19, 2026

View reviewed changes

Comment thread src/hiero_sdk_python/client/network.py

Mounil2005 marked this pull request as draft July 19, 2026 21:53

iron-prog force-pushed the fix-2403-print-to-logger-v2 branch from 6f0fa5d to 2e7e5ac Compare July 20, 2026 03:32

iron-prog marked this pull request as ready for review July 21, 2026 15:38

aceppaluni added the status: update branch developer needs to click update branch label Jul 21, 2026

aceppaluni requested review from danielmarv and manishdait July 21, 2026 16:39

aceppaluni added reviewer: maintainer PR needs a review from the maintainer team reviewer: committer request review help from a committer labels Jul 21, 2026

manishdait changed the title ~~Fix 2403 print to logger v2~~ chore: Replace print statements with logger Jul 22, 2026

manishdait requested changes Jul 22, 2026

View reviewed changes

exploreriii approved these changes Jul 22, 2026

View reviewed changes

github-actions Bot added queue:committers PR awaiting committer technical review and removed queue:junior-committer PR awaiting initial quality review labels Jul 22, 2026

iron-prog added 4 commits July 22, 2026 19:10

Replace print statements with structured logging

8db08bb

Signed-off-by: iron-prog <dt915725@gmail.com>

Add regression tests for structured logging

6215311

Signed-off-by: iron-prog <dt915725@gmail.com>

fix: catch malformed mirror node response data

24472d2

Signed-off-by: iron-prog <dt915725@gmail.com>

iron-prog force-pushed the fix-2403-print-to-logger-v2 branch from 2e7e5ac to 24472d2 Compare July 22, 2026 13:41

coderabbitai Bot reviewed Jul 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: Replace print statements with logger#2452

chore: Replace print statements with logger#2452
iron-prog wants to merge 4 commits into
hiero-ledger:mainfrom
iron-prog:fix-2403-print-to-logger-v2

iron-prog commented Jul 18, 2026

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

manishdait left a comment

Uh oh!

manishdait Jul 22, 2026

Uh oh!

exploreriii Jul 22, 2026

Uh oh!

exploreriii Jul 22, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 22, 2026

Uh oh!

coderabbitai Bot Jul 22, 2026

Uh oh!

coderabbitai Bot Jul 22, 2026

Uh oh!

coderabbitai Bot Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		source = inspect.getsource(Network._fetch_nodes_from_mirror_node)
		assert "print(" not in source

Uh oh!

Conversation

iron-prog commented Jul 18, 2026

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

Uh oh!

manishdait left a comment

Choose a reason for hiding this comment

Uh oh!

manishdait Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

exploreriii Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

exploreriii Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading