test: report detailed msg during utf8 response decoding error

furszy commented at 7:06 pm on November 7, 2024: member

Useful for debugging issues such #31241 (comment).

Prints the entire response content instead of printing only the position of the byte it can’t be decoded.

The diff between the error messages can be seen by running the wallet_migration.py functional test with the following patch applied:

 0diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
 1--- a/src/wallet/rpc/wallet.cpp	(revision d65918c5da52c7d5035b4151dee9ffb2e94d4761)
 2+++ b/src/wallet/rpc/wallet.cpp	(date 1731005254673)
 3@@ -801,7 +801,7 @@
 4             }
 5 
 6             UniValue r{UniValue::VOBJ};
 7-            r.pushKV("wallet_name", res->wallet_name);
 8+            r.pushKV("wallet_name", "\xc3\x28");
 9             if (res->watchonly_wallet) {
10                 r.pushKV("watchonly_name", res->watchonly_wallet->GetName());
11             }

DrahtBot commented at 7:06 pm on November 7, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31251.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	theStack, rkrux

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Tests on Nov 7, 2024

furszy renamed this:
~~test: report a more detailed failure during utf8 response decoding~~
test: report more detailed failure during utf8 response decoding
on Nov 7, 2024

furszy renamed this:
~~test: report more detailed failure during utf8 response decoding~~
test: report detailed msg during utf8 response decoding error
on Nov 8, 2024

in test/functional/test_framework/authproxy.py:196 in d65918c5da outdated

192+        data = http_response.read()
193+        try:
194+            responsedata = data.decode('utf8')
195+        except UnicodeDecodeError:
196+            raise JSONRPCException({
197+                'code': -342, 'message': f'Cannot decode response in utf8 format, content: {data}'})

theStack commented at 11:58 am on November 11, 2024:

nit: it might be helpful to still include the original exception message with the decoding error offset and first invalid byte, e.g.

0        except UnicodeDecodeError as e:
1            raise JSONRPCException({
2                'code': -342, 'message': f'Cannot decode response in utf8 format, exception: {e}, content: {data}'})

furszy commented at 3:46 pm on November 11, 2024:

sure, done as suggested. Thanks

theStack approved

theStack commented at 12:01 pm on November 11, 2024: contributor

Tested ACK d65918c5da52c7d5035b4151dee9ffb2e94d4761

Makes sense to provide more detailed information in case of a decoding error.

master:

0...
1    responsedata = http_response.read().decode('utf8')
2UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc3 in position 42: invalid continuation byte
3...

PR:

0...
1test_framework.authproxy.JSONRPCException: Cannot decode response in utf8 format, content: b'{"jsonrpc":"2.0","result":{"wallet_name":"\xc3(","backup_path":"/tmp/bitcoin_func_test_eqgkvabr/node0/regtest/wallets/basic0/basic0_1731326134.legacy.bak"},"id":22}\n' (-342)
2...

test: report failure during utf8 response decoding

Useful for debugging issues.

a2c45ae548

furszy force-pushed on Nov 11, 2024

theStack approved

theStack commented at 4:12 pm on November 11, 2024: contributor

re-ACK a2c45ae5480a2ee643665d6ecaee9714a287a70e

in test/functional/test_framework/authproxy.py:196 in a2c45ae548

192+        data = http_response.read()
193+        try:
194+            responsedata = data.decode('utf8')
195+        except UnicodeDecodeError as e:
196+            raise JSONRPCException({
197+                'code': -342, 'message': f'Cannot decode response in utf8 format, content: {data}, exception: {e}'})

rkrux commented at 10:46 am on November 12, 2024:

A nit but it’s easier to read as well, catches eyes quickly.

0                'code': -342, 'message': f'Cannot decode response in UTF-8 format, content: {data}, exception: {e}'})

rkrux commented at 10:50 am on November 12, 2024:

content: {data}

Out of curiosity: Do you think we should limit printing the data here to a certain length or printing all of it is fine as well? Idk atm if the RPC responses from core have a size limit.

furszy commented at 3:20 pm on November 12, 2024:

Out of curiosity: Do you think we should limit printing the data here to a certain length or printing all of it is fine as well? Idk atm if the RPC responses from core have a size limit.

Should check the limit, there shouldn’t be any. But, as this is an error message, which shouldn’t usually happen, I think that it is better to receive the entire content.

rkrux approved

rkrux commented at 10:54 am on November 12, 2024: none

tACK a2c45ae5480a2ee643665d6ecaee9714a287a70e

Certainly an improvement in understanding failures. I applied the suggested patch and compiled core with BDB wallet. Following is the trace at my end:

 02024-11-12T10:15:36.800000Z TestFramework (INFO): Test migration of a basic keys only wallet without balance
 12024-11-12T10:15:38.407000Z TestFramework (ERROR): JSONRPC error
 2Traceback (most recent call last):
 3  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/authproxy.py", line 193, in _get_response
 4    responsedata = data.decode('utf8')
 5UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc3 in position 42: invalid continuation byte
 6
 7During handling of the above exception, another exception occurred:
 8
 9Traceback (most recent call last):
10  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/test_framework.py", line 132, in main
11    self.run_test()
12    ~~~~~~~~~~~~~^^
13  File "/Users/rkrux/projects/bitcoin/build/test/functional/wallet_migration.py", line 1020, in run_test
14    self.test_basic()
15    ~~~~~~~~~~~~~~~^^
16  File "/Users/rkrux/projects/bitcoin/build/test/functional/wallet_migration.py", line 119, in test_basic
17    self.migrate_wallet(basic0)
18    ~~~~~~~~~~~~~~~~~~~^^^^^^^^
19  File "/Users/rkrux/projects/bitcoin/build/test/functional/wallet_migration.py", line 73, in migrate_wallet
20    return wallet_rpc.migratewallet(*args, **kwargs)
21           ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
22  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
23    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
24  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/authproxy.py", line 127, in __call__
25    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
26                       ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/authproxy.py", line 106, in _request
28    return self._get_response()
29           ~~~~~~~~~~~~~~~~~~^^
30  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/authproxy.py", line 195, in _get_response
31    raise JSONRPCException({
32        'code': -342, 'message': f'Cannot decode response in utf8 format, content: {data}, exception: {e}'})
33test_framework.authproxy.JSONRPCException: Cannot decode response in utf8 format, content: b'{"jsonrpc":"2.0","result":{"wallet_name":"\xc3(","backup_path":"/var/folders/6v/mpspb4bx4zzf3xr2z96hgq9h0000gn/T/bitcoin_func_test__r03c8f0/node0/regtest/wallets/basic0/basic0_1731406538.legacy.bak"},"id":23}\n', exception: 'utf-8' codec can't decode byte 0xc3 in position 42: invalid continuation byte (-342)
342024-11-12T10:15:38.468000Z TestFramework (INFO): Stopping nodes

test: report detailed msg during utf8 response decoding error #31251

Code Coverage & Benchmarks

Reviews