Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Chain service error #989

Merged
merged 9 commits into from
May 27, 2024
Merged

Chain service error #989

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c59dd37
Check chain service response status codes.
roeierez May 26, 2024
cb22709
log error on chain service failures
roeierez May 26, 2024
e5f7698
Allow HTTP status code check in GET response parsing
ok300 May 27, 2024
f9ecc4e
Move payload parsing after the HTTP code status check
ok300 May 27, 2024
ad08487
Merge pull request #990 from breez/ok300-chain-service-error-v2
ok300 May 27, 2024
3830eb6
Merge branch 'main' into chain-service-error
ok300 May 27, 2024
65e6079
Add test for lnurl failures with non 200 status code
roeierez May 27, 2024
8b14292
simplify code
roeierez May 27, 2024
1209a8f
fix clippy
roeierez May 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions libs/sdk-core/src/input_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1075,15 +1075,24 @@ pub(crate) mod tests {
}
"#.replace('\n', "");

let response_body = match return_lnurl_error {
let response_body = match &return_lnurl_error {
None => expected_lnurl_withdraw_data,
Some(err_reason) => {
["{\"status\": \"ERROR\", \"reason\": \"", &err_reason, "\"}"].join("")
}
};

let status = match &return_lnurl_error {
None => 200,
Some(_) => 400,
};

Copy link
Contributor

@erdemyerebasmaz erdemyerebasmaz May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can match once and return the result as tuple here:

let (response_body, status) = match &return_lnurl_error {
    None => (expected_lnurl_withdraw_data, 200),
    Some(err_reason) => (
        ["{\"status\": \"ERROR\", \"reason\": \"", &err_reason, "\"}"].join(""),
        400,
    ),
};

or we can use response_body to set status to avoid matching twice:

let status = if return_lnurl_error.is_none() {
    200
} else {
    400
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roeierez cargo clippy has this warning:

error: this expression creates a reference which is immediately dereferenced by the compiler
    --> sdk-core/src/input_parser.rs:1081:60
     |
1081 |                 ["{\"status\": \"ERROR\", \"reason\": \"", &err_reason, "\"}"].join(""),
     |                                                            ^^^^^^^^^^^ help: change this to: `err_reason`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
     = note: `-D clippy::needless-borrow` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(clippy::needless_borrow)]`

let mut server = MOCK_HTTP_SERVER.lock().unwrap();
server.mock("GET", path).with_body(response_body).create()
server
.mock("GET", path)
.with_body(response_body)
.with_status(status)
.create()
}

#[tokio::test]
Expand Down
Loading