Skip to content

fix(log): suppress spurious error logs when solidity node exits#6801

Open
317787106 wants to merge 1 commit into
tronprotocol:release_v4.8.2from
317787106:hotfix/solidity_log
Open

fix(log): suppress spurious error logs when solidity node exits#6801
317787106 wants to merge 1 commit into
tronprotocol:release_v4.8.2from
317787106:hotfix/solidity_log

Conversation

@317787106
Copy link
Copy Markdown
Collaborator

@317787106 317787106 commented May 25, 2026

What does this PR do?

  • Suppress spurious ERROR ... reason: null logs when the solidity node shuts down.

Root cause

close() calls getBlockExecutor.shutdownNow() while the get-block thread may be blocked in blockQueue.put(). That throws InterruptedException, whose getMessage() returns null. The original catch (Exception e) logged it as an ERROR with reason: null.

The original sleep() had the same issue: it caught Exception instead of InterruptedException, logged a null message, and never restored the interrupt flag.

Changes

Location Change
getBlock() Add catch (InterruptedException e) — restore interrupt flag, log INFO, return
getBlock() Add !flag guard in catch (Exception e) — log INFO instead of ERROR on shutdown
getBlockByNum() Null-check gRPC response to prevent NPE during channel teardown
getLastSolidityBlockNum() Add !flag guard — same pattern as getBlock()
sleep() Catch InterruptedException specifically and restore interrupt status

@github-actions github-actions Bot requested a review from halibobo1205 May 25, 2026 07:56
@317787106 317787106 changed the title fix(log): don't log error when solidity node exist fix(log): suppress spurious error logs when solidity node exits May 25, 2026
@317787106 317787106 force-pushed the hotfix/solidity_log branch from 73567d3 to 14b4d46 Compare May 25, 2026 10:06
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone May 25, 2026
try {
long time = System.currentTimeMillis();
Block block = databaseGrpcClient.getBlock(blockNum);
if (block == null) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at DatabaseGrpcClient.getBlock(), it delegates to databaseBlockingStub.getBlockByNum(...) — a gRPC blocking stub. Per gRPC semantics, blocking stubs don't return null: success returns a response message (the default instance is still a non-null object), and channel teardown/cancellation/deadline expiry surfaces as StatusRuntimeException (CANCELLED / UNAVAILABLE / DEADLINE_EXCEEDED), not null.

A few questions:

  1. Have we observed this null path being hit in production? If so, what's the server-side implementation that produces it?
  2. If this is purely defensive (against mocks or future stub changes), could we add an inline comment? Future readers will spend time looking for "when does this return null."

Copy link
Copy Markdown
Collaborator Author

@317787106 317787106 May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it is defensive. Per gRPC semantics, a blocking stub call never returns null — on channel teardown it throws StatusRuntimeException (e.g. CANCELLED), which is already caught by the outer catch (Exception e) block and logged as INFO via the !flag guard. Removed the unreachable null check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@317787106 317787106 force-pushed the hotfix/solidity_log branch from 14b4d46 to d1ecbc1 Compare May 25, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants