Avoid ugly exception in log on unknown inv type #9112

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2016_11_invtype_debugging changing 1 files +5 −1

laanwj commented at 10:16 AM on November 9, 2016: member
It is unexpected behavior for ToString to raise an exception. It is expected to do a best-effort attempt at formatting but never fail.

Catch the exception and simply print unknown inv types as hexadecimal.

Before:
```
************************
EXCEPTION: St12out_of_range       
CInv::GetCommand(): type=666 unknown type       
bitcoin in ProcessMessages()       
```
After:
```
2016-11-09 10:08:20 got inv: 0x0000029a 0000000000000000000000000000000000000000000000000000000000000000  have peer=1
```
Fixes #9110.
laanwj added the label Docs and Output on Nov 9, 2016

Avoid ugly exception in log on unknown inv type

It is unexpected behavior for `ToString` to raise an exception. It
is expected to do a best-effort attempt at formatting but never fail.

Catch the exception and simply print unknown inv types as hexadecimal.

Fixes #9110.

e9f25ddd00

laanwj force-pushed on Nov 9, 2016

in src/protocol.cpp:None in e9f25ddd00

 180 | @@ -181,7 +181,11 @@ std::string CInv::GetCommand() const
 181 |  
 182 |  std::string CInv::ToString() const
 183 |  {
 184 | -    return strprintf("%s %s", GetCommand(), hash.ToString());
 185 | +    try {
 186 | +        return strprintf("%s %s", GetCommand(), hash.ToString());
 187 | +    } catch(const std::out_of_range &) {
 188 | +        return strprintf("0x%08x %s", type, hash.ToString());

rebroad commented at 10:17 AM on November 9, 2016:

perfect

rebroad commented at 10:21 AM on November 9, 2016:

is it worth changing the GetCommand() function to return this string for the default case also?

paveljanik commented at 10:24 AM on November 9, 2016: contributor

I'd prefer try first and then one final line with return, ie. one exit route even if this is very simple.
laanwj commented at 10:30 AM on November 9, 2016: member

Can we just decide to merge this (or not) instead of bombarding a two-line logging only change to death with bikeshedding?
paveljanik commented at 10:32 AM on November 9, 2016: contributor

NACK I prefer the other solution #9113. No need to throw an exception at all. It also fixes the problem for other callers.
laanwj commented at 10:35 AM on November 9, 2016: member

Fucking damnit do we really need two pulls for this non issue
theuni commented at 2:58 PM on November 9, 2016: member

utACK e9f25ddd0063f7ea65595178d04370d27344305c. Safer to fix it at the site of the problem for now.
fanquake commented at 9:55 AM on November 11, 2016: member

utACK e9f25dd
laanwj merged this on Nov 11, 2016
laanwj closed this on Nov 11, 2016
laanwj referenced this in commit 46027e8668 on Nov 11, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#9110 CInv::GetCommand(): type=1373179482 unknown type #9113 Return the type when it's unknown instead of throwing an exception