Skip to content

Commit 47dad42

Browse files
fanquakefanquake
committed
Merge bitcoin/bitcoin#25629: univalue: Return more detailed type check error messages
fae5ce8 univalue: Return more detailed type check error messages (MacroFake) fafab14 move-only: Move UniValue::getInt definition to keep class with definitions only (MacroFake) Pull request description: Print the current type and the expected type ACKs for top commit: aureleoules: ACK fae5ce8. Tree-SHA512: 4ae720a012ff8245baf5cd7f844f93b946c58feebe62de6dfd84ebc5c8afb988295a94de7c01aef98aaf4c6228f7184ed622f37079c738924617e0f336ac5b6e
2 parents 6d8707b + fae5ce8 commit 47dad42

File tree

11 files changed

+54
-50
lines changed

11 files changed

+54
-50
lines changed

‎src/univalue/include/univalue.h

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ class UniValue {
106106
std::vector<std::string> keys;
107107
std::vector<UniValue> values;
108108

109+
void checkType(const VType& expected) const;
109110
bool findKey(const std::string& key, size_t& retIdx) const;
110111
void writeArray(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const;
111112
void writeObject(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const;
@@ -116,19 +117,7 @@ class UniValue {
116117
const std::vector<std::string>& getKeys() const;
117118
const std::vector<UniValue>& getValues() const;
118119
template <typename Int>
119-
auto getInt() const
120-
{
121-
static_assert(std::is_integral<Int>::value);
122-
if (typ != VNUM) {
123-
throw std::runtime_error("JSON value is not an integer as expected");
124-
}
125-
Int result;
126-
const auto [first_nonmatching, error_condition] = std::from_chars(val.data(), val.data() + val.size(), result);
127-
if (first_nonmatching != val.data() + val.size() || error_condition != std::errc{}) {
128-
throw std::runtime_error("JSON integer out of range");
129-
}
130-
return result;
131-
}
120+
Int getInt() const;
132121
bool get_bool() const;
133122
const std::string& get_str() const;
134123
double get_real() const;
@@ -142,10 +131,23 @@ class UniValue {
142131
template <class It>
143132
void UniValue::push_backV(It first, It last)
144133
{
145-
if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
134+
checkType(VARR);
146135
values.insert(values.end(), first, last);
147136
}
148137

138+
template <typename Int>
139+
Int UniValue::getInt() const
140+
{
141+
static_assert(std::is_integral<Int>::value);
142+
checkType(VNUM);
143+
Int result;
144+
const auto [first_nonmatching, error_condition] = std::from_chars(val.data(), val.data() + val.size(), result);
145+
if (first_nonmatching != val.data() + val.size() || error_condition != std::errc{}) {
146+
throw std::runtime_error("JSON integer out of range");
147+
}
148+
return result;
149+
}
150+
149151
enum jtokentype {
150152
JTOK_ERR = -1,
151153
JTOK_NONE = 0, // eof

‎src/univalue/lib/univalue.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,29 +110,29 @@ bool UniValue::setObject()
110110

111111
void UniValue::push_back(const UniValue& val_)
112112
{
113-
if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
113+
checkType(VARR);
114114

115115
values.push_back(val_);
116116
}
117117

118118
void UniValue::push_backV(const std::vector<UniValue>& vec)
119119
{
120-
if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
120+
checkType(VARR);
121121

122122
values.insert(values.end(), vec.begin(), vec.end());
123123
}
124124

125125
void UniValue::__pushKV(const std::string& key, const UniValue& val_)
126126
{
127-
if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
127+
checkType(VOBJ);
128128

129129
keys.push_back(key);
130130
values.push_back(val_);
131131
}
132132

133133
void UniValue::pushKV(const std::string& key, const UniValue& val_)
134134
{
135-
if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
135+
checkType(VOBJ);
136136

137137
size_t idx;
138138
if (findKey(key, idx))
@@ -143,7 +143,8 @@ void UniValue::pushKV(const std::string& key, const UniValue& val_)
143143

144144
void UniValue::pushKVs(const UniValue& obj)
145145
{
146-
if (typ != VOBJ || obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
146+
checkType(VOBJ);
147+
obj.checkType(VOBJ);
147148

148149
for (size_t i = 0; i < obj.keys.size(); i++)
149150
__pushKV(obj.keys[i], obj.values.at(i));
@@ -213,6 +214,14 @@ const UniValue& UniValue::operator[](size_t index) const
213214
return values.at(index);
214215
}
215216

217+
void UniValue::checkType(const VType& expected) const
218+
{
219+
if (typ != expected) {
220+
throw std::runtime_error{"JSON value of type " + std::string{uvTypeName(typ)} + " is not of expected type " +
221+
std::string{uvTypeName(expected)}};
222+
}
223+
}
224+
216225
const char *uvTypeName(UniValue::VType t)
217226
{
218227
switch (t) {

‎src/univalue/lib/univalue_get.cpp

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ bool ParseDouble(const std::string& str, double *out)
4646

4747
const std::vector<std::string>& UniValue::getKeys() const
4848
{
49-
if (typ != VOBJ)
50-
throw std::runtime_error("JSON value is not an object as expected");
49+
checkType(VOBJ);
5150
return keys;
5251
}
5352

@@ -60,22 +59,19 @@ const std::vector<UniValue>& UniValue::getValues() const
6059

6160
bool UniValue::get_bool() const
6261
{
63-
if (typ != VBOOL)
64-
throw std::runtime_error("JSON value is not a boolean as expected");
62+
checkType(VBOOL);
6563
return getBool();
6664
}
6765

6866
const std::string& UniValue::get_str() const
6967
{
70-
if (typ != VSTR)
71-
throw std::runtime_error("JSON value is not a string as expected");
68+
checkType(VSTR);
7269
return getValStr();
7370
}
7471

7572
double UniValue::get_real() const
7673
{
77-
if (typ != VNUM)
78-
throw std::runtime_error("JSON value is not a number as expected");
74+
checkType(VNUM);
7975
double retval;
8076
if (!ParseDouble(getValStr(), &retval))
8177
throw std::runtime_error("JSON double out of range");
@@ -84,15 +80,12 @@ double UniValue::get_real() const
8480

8581
const UniValue& UniValue::get_obj() const
8682
{
87-
if (typ != VOBJ)
88-
throw std::runtime_error("JSON value is not an object as expected");
83+
checkType(VOBJ);
8984
return *this;
9085
}
9186

9287
const UniValue& UniValue::get_array() const
9388
{
94-
if (typ != VARR)
95-
throw std::runtime_error("JSON value is not an array as expected");
89+
checkType(VARR);
9690
return *this;
9791
}
98-

‎test/functional/mining_prioritisetransaction.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ def run_test(self):
122122

123123
# Test `prioritisetransaction` invalid `dummy`
124124
txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000'
125-
assert_raises_rpc_error(-1, "JSON value is not a number as expected", self.nodes[0].prioritisetransaction, txid, 'foo', 0)
125+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", self.nodes[0].prioritisetransaction, txid, 'foo', 0)
126126
assert_raises_rpc_error(-8, "Priority is no longer supported, dummy argument to prioritisetransaction must be 0.", self.nodes[0].prioritisetransaction, txid, 1, 0)
127127

128128
# Test `prioritisetransaction` invalid `fee_delta`
129-
assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo')
129+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo')
130130

131131
self.test_diamond()
132132

‎test/functional/rpc_blockchain.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,12 +261,12 @@ def _test_getchaintxstats(self):
261261
assert_raises_rpc_error(-1, 'getchaintxstats', self.nodes[0].getchaintxstats, 0, '', 0)
262262

263263
# Test `getchaintxstats` invalid `nblocks`
264-
assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].getchaintxstats, '')
264+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", self.nodes[0].getchaintxstats, '')
265265
assert_raises_rpc_error(-8, "Invalid block count: should be between 0 and the block's height - 1", self.nodes[0].getchaintxstats, -1)
266266
assert_raises_rpc_error(-8, "Invalid block count: should be between 0 and the block's height - 1", self.nodes[0].getchaintxstats, self.nodes[0].getblockcount())
267267

268268
# Test `getchaintxstats` invalid `blockhash`
269-
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getchaintxstats, blockhash=0)
269+
assert_raises_rpc_error(-1, "JSON value of type number is not of expected type string", self.nodes[0].getchaintxstats, blockhash=0)
270270
assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 1, for '0')", self.nodes[0].getchaintxstats, blockhash='0')
271271
assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getchaintxstats, blockhash='ZZZ0000000000000000000000000000000000000000000000000000000000000')
272272
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0000000000000000000000000000000000000000000000000000000000000000')
@@ -536,7 +536,7 @@ def assert_vin_does_not_contain_prevout(verbosity):
536536
datadir = get_datadir_path(self.options.tmpdir, 0)
537537

538538
self.log.info("Test getblock with invalid verbosity type returns proper error message")
539-
assert_raises_rpc_error(-1, "JSON value is not an integer as expected", node.getblock, blockhash, "2")
539+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", node.getblock, blockhash, "2")
540540

541541
def move_block_file(old, new):
542542
old_path = os.path.join(datadir, self.chain, 'blocks', old)

‎test/functional/rpc_fundrawtransaction.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ def test_change_type(self):
301301
inputs = [ {'txid' : utx['txid'], 'vout' : utx['vout']} ]
302302
outputs = { self.nodes[0].getnewaddress() : Decimal(4.0) }
303303
rawtx = self.nodes[2].createrawtransaction(inputs, outputs)
304-
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[2].fundrawtransaction, rawtx, {'change_type': None})
304+
assert_raises_rpc_error(-1, "JSON value of type null is not of expected type string", self.nodes[2].fundrawtransaction, rawtx, {'change_type': None})
305305
assert_raises_rpc_error(-5, "Unknown change type ''", self.nodes[2].fundrawtransaction, rawtx, {'change_type': ''})
306306
rawtx = self.nodes[2].fundrawtransaction(rawtx, {'change_type': 'bech32'})
307307
dec_tx = self.nodes[2].decoderawtransaction(rawtx['hex'])

‎test/functional/rpc_help.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ def test_categories(self):
9292
assert_raises_rpc_error(-1, 'help', node.help, 'foo', 'bar')
9393

9494
# invalid argument
95-
assert_raises_rpc_error(-1, 'JSON value is not a string as expected', node.help, 0)
95+
assert_raises_rpc_error(-1, "JSON value of type number is not of expected type string", node.help, 0)
9696

9797
# help of unknown command
9898
assert_equal(node.help('foo'), 'help: unknown command: foo')

‎test/functional/rpc_rawtransaction.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,13 @@ def getrawtransaction_tests(self):
124124

125125
# 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose
126126
for value in ["True", "False"]:
127-
assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
127+
assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
128128

129129
# 7. invalid parameters - supply txid and empty array
130-
assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, [])
130+
assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransaction, txId, [])
131131

132132
# 8. invalid parameters - supply txid and empty dict
133-
assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {})
133+
assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransaction, txId, {})
134134

135135
# Make a tx by sending, then generate 2 blocks; block1 has the tx in it
136136
tx = self.wallet.send_self_transfer(from_node=self.nodes[2])['txid']
@@ -152,7 +152,7 @@ def getrawtransaction_tests(self):
152152
# We should not get the tx if we provide an unrelated block
153153
assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2)
154154
# An invalid block hash should raise the correct errors
155-
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True)
155+
assert_raises_rpc_error(-1, "JSON value of type bool is not of expected type string", self.nodes[n].getrawtransaction, txid=tx, blockhash=True)
156156
assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar")
157157
assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234")
158158
foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000"
@@ -181,8 +181,8 @@ def createrawtransaction_tests(self):
181181

182182
# Test `createrawtransaction` invalid `inputs`
183183
assert_raises_rpc_error(-3, "Expected type array", self.nodes[0].createrawtransaction, 'foo', {})
184-
assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {})
185-
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {})
184+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type object", self.nodes[0].createrawtransaction, ['foo'], {})
185+
assert_raises_rpc_error(-1, "JSON value of type null is not of expected type string", self.nodes[0].createrawtransaction, [{}], {})
186186
assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {})
187187
txid = "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844"
188188
assert_raises_rpc_error(-8, f"txid must be hexadecimal string (not '{txid}')", self.nodes[0].createrawtransaction, [{'txid': txid}], {})
@@ -207,7 +207,7 @@ def createrawtransaction_tests(self):
207207

208208
# Test `createrawtransaction` invalid `outputs`
209209
address = getnewdestination()[2]
210-
assert_raises_rpc_error(-1, "JSON value is not an array as expected", self.nodes[0].createrawtransaction, [], 'foo')
210+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type array", self.nodes[0].createrawtransaction, [], 'foo')
211211
self.nodes[0].createrawtransaction(inputs=[], outputs={}) # Should not throw for backwards compatibility
212212
self.nodes[0].createrawtransaction(inputs=[], outputs=[])
213213
assert_raises_rpc_error(-8, "Data must be hexadecimal string", self.nodes[0].createrawtransaction, [], {'data': 'foo'})

‎test/functional/wallet_basic.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ def run_test(self):
414414
assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "1f-4")
415415

416416
# This will raise an exception since generate does not accept a string
417-
assert_raises_rpc_error(-1, "not an integer", self.generate, self.nodes[0], "2")
417+
assert_raises_rpc_error(-1, "not of expected type number", self.generate, self.nodes[0], "2")
418418

419419
if not self.options.descriptors:
420420

‎test/functional/wallet_hd.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ def run_test(self):
173173
# Sethdseed parameter validity
174174
assert_raises_rpc_error(-1, 'sethdseed', self.nodes[0].sethdseed, False, new_seed, 0)
175175
assert_raises_rpc_error(-5, "Invalid private key", self.nodes[1].sethdseed, False, "not_wif")
176-
assert_raises_rpc_error(-1, "JSON value is not a boolean as expected", self.nodes[1].sethdseed, "Not_bool")
177-
assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[1].sethdseed, False, True)
176+
assert_raises_rpc_error(-1, "JSON value of type string is not of expected type bool", self.nodes[1].sethdseed, "Not_bool")
177+
assert_raises_rpc_error(-1, "JSON value of type bool is not of expected type string", self.nodes[1].sethdseed, False, True)
178178
assert_raises_rpc_error(-5, "Already have this key", self.nodes[1].sethdseed, False, new_seed)
179179
assert_raises_rpc_error(-5, "Already have this key", self.nodes[1].sethdseed, False, self.nodes[1].dumpprivkey(self.nodes[1].getnewaddress()))
180180

0 commit comments

Comments
 (0)