ethstats: added some error handling in ethstats.go #29834
Conversation
ethstats/ethstats.go
Outdated
| @@ -610,6 +610,7 @@ func (s *Service) reportBlock(conn *connWrapper, block *types.Block) error { | |||
| if details == nil { | |||
| return nil | |||
| } | |||
|
|
|||
pratikspatil024
force-pushed
the
fix-panic
branch
from
d9a042f to
69b496b
Compare
|
The PR doesn't meaningfully improve the logic. The ethstats module will try to access the latest block repeatedly. If there is an error accessing the database, we do not really want to see it because there will be lots of other reports of the access failure already. |
| block, err = fullBackend.BlockByNumber(context.Background(), rpc.BlockNumber(head.Number.Uint64())) | ||
| // Short circuit if no block is available. It might happen when | ||
| // the blockchain is reorging. | ||
| if err != nil { |
There was a problem hiding this comment.
Keeping the old check of block == nil seems safer as it will guarantee that we won't crash later when accessing it. I don't particularly mind retrieving and printing teh error, but would keep the check.
There was a problem hiding this comment.
@karalabe Its the same thing, right? If there is an error fetching the block, the the block returned will be nil and we are returning nil. If the error is not nil, that means the we have a block which is not nil.
But I agree with @fjl :
The PR doesn't meaningfully improve the logic
I can change the log level to DEBUG, but upto you guys to decide. Thanks.
Although the panic was already addressed in PR#29020, I thought of catching the error and modifying the approach a little bit.
I am currently throwing an
ERROR, but that might be too harsh, so can change it toWARN.