bitreq: Re-implement `json-using-serde` feature by tcharding · Pull Request #398 · rust-bitcoin/corepc

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding · 2025-10-26T23:25:06Z

This feature was removed when we gutted minreq after forking it. Turns out we need it to be able to use bitreq in jsonrpc.

Re-implement the feature by copying code from minreq.

Close: #393

jamillambert · 2025-10-29T13:55:35Z

+ /// ```no_run
+ /// use serde_json::Value;
+ ///
+ /// # fn main() -> Result<(), minreq::Error> {


Suggested change
/// # fn main() -> Result<(), minreq::Error> {
/// # fn main() -> Result<(), bitreq::Error> {

jamillambert · 2025-10-29T13:55:47Z

+ /// # fn main() -> Result<(), minreq::Error> {
+ /// # let url_to_json_resource = "http://example.org/resource.json";
+ /// // Value could be any type that implements Deserialize!
+ /// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;


Suggested change
/// let user = minreq::get(url_to_json_resource).send()?.json::<Value>()?;
/// let user = bitreq::get(url_to_json_resource).send()?.json::<Value>()?;

Why are these not caught in CI? I get an error locally. Ah, is it because of #400 not being merged yet?

jamillambert · 2025-10-29T14:02:38Z

+ let original_json: serde_json::Value = serde_json::from_str(JSON_SRC).unwrap();
+ let response = bitreq::post(url("/echo")).with_json(&original_json).unwrap().send().unwrap();
+ let actual_json: serde_json::Value = response.json().unwrap();
+ assert_eq!(&actual_json, &original_json);


Suggested change
assert_eq!(&actual_json,&original_json);
assert_eq!(actual_json, original_json);

jamillambert · 2025-10-29T14:23:53Z

+ let str = match self.as_str() {
+ Ok(str) => str,
+ Err(_) => return Err(Error::InvalidUtf8InResponse),
+ };
+ match serde_json::from_str(str) {
+ Ok(json) => Ok(json),
+ Err(err) => Err(Error::SerdeJsonError(err)),


Suggested change
let str = matchself.as_str(){
Ok(str) => str,
Err(_) => returnErr(Error::InvalidUtf8InResponse),
};
match serde_json::from_str(str){
Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
serde_json::from_str(self.as_str()?)
.map_err(Error::SerdeJsonError)
The existing code returns the incorrect error variant. The docs say InvalidUtf8InBody which is what as_str() returns.

Half right I think. Your suggestion is correct and better but AFAICT it doesn't change the logic. I used the suggestion.

This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. After copy, fix docs to mention `bitreq` and also refactor some error logic to use `as_str()?` directly (no logic change).

oleonardolima

tACK 99c948c

I've tested it locally, and with the rust-esplora-client PR (see bitcoindevkit/rust-esplora-client#137), and it works just fine.

jamillambert

ACK 99c948c

99c948c Re-implement json-using-serde feature (Tobin C. Harding) 1251953 Replace stale docs reference to minreq (Tobin C. Harding) Pull request description: This feature was removed when we gutted `minreq` after forking it. Turns out we need it to be able to use `bitreq` in `jsonrpc`. Re-implement the feature by copying code from `minreq`. Close: rust-bitcoin#393 ACKs for top commit: oleonardolima: tACK 99c948c jamillambert: ACK 99c948c Tree-SHA512: 2e22881566fd3d75e0edb70567dac0b1ed9ce66720511dcead888b2b7686fc89fb1f6bce3bd490f91997e9ade88b0665b00679c79bcef35205c6ce1117306faf

tcharding force-pushed the push-rvqmqwwmqtvm branch from e45e36b to 722ee7aCompare October 26, 2025 23:31

tcharding mentioned this pull request Oct 26, 2025
jsonrpc: Depend on bitreq#399
Merged

tcharding force-pushed the push-rvqmqwwmqtvm branch from 69deb6d to be01458Compare October 26, 2025 23:42

tcharding changed the title ~~Re-implement json-using-serde feature~~bitreq: Re-implement json-using-serde featureOct 27, 2025

jamillambert reviewed Oct 29, 2025
View reviewed changes

tcharding added 2 commits October 30, 2025 09:25

Replace stale docs reference to minreq
1251953

tcharding force-pushed the push-rvqmqwwmqtvm branch from be01458 to 99c948cCompare October 29, 2025 22:25

oleonardolima reviewed Oct 30, 2025
View reviewed changes

jamillambert approved these changes Oct 30, 2025
View reviewed changes

tcharding merged commit b68f20d into rust-bitcoin:masterOct 30, 2025
30 checks passed

tcharding mentioned this pull request Oct 30, 2025
Integrate bitreq crate into CI tests #400
Merged