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.

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

FIX eclair-cli error code in case of HTTP problem #2798

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

pgrange
Copy link
Contributor

@pgrange pgrange commented Dec 8, 2023

Last command of eclair-cli is a curl piped to jq. In case of an error with the curl command (for instance, eclair service is not running) the error code is swallowed by the pipe which will return the exit code of the last command of the pipe (here, jq).

Setting pipefail ensures that the error code of curl is preserved in case of HTTP issue.

This can come handy, for instance, to check that eclair is not ok:

if eclair-cli getinfo
then
  echo eclair is answering
else
  echo problem communicating with eclair
fi

Last command of eclair-cli is a curl piped to jq. In case of an
error with the curl command (for instance, eclair service is not
running) the error code was swallowed by the pipe wich will
return the exit code of the last command of the pipe (here, jq).

Setting `pipefail` ensures that the error code of curl is
preserve in case of HTTP issue.

This can come handy, for instance, to check that eclair is not
ok:

```
if eclair-cli getinfo
then
  echo eclair is answering
else
  echo problem communicating with eclair
fi
```
@t-bast t-bast requested a review from sstone December 8, 2023 08:13
@t-bast
Copy link
Member

t-bast commented Dec 8, 2023

Thanks! @sstone can you take a look at this?

@codecov-commenter
Copy link

Codecov Report

Merging #2798 (6b76fa5) into master (be4ed3c) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2798      +/-   ##
==========================================
+ Coverage   85.86%   85.87%   +0.01%     
==========================================
  Files         216      216              
  Lines       18177    18177              
  Branches      786      786              
==========================================
+ Hits        15607    15609       +2     
+ Misses       2570     2568       -2     

see 5 files with indirect coverage changes

Copy link
Member

@sstone sstone left a comment

Choose a reason for hiding this comment

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

Thanks! tested on ubuntu and AWS linux (but it should not break any working setup since we specify #!/bin/bash at the beginning of eclair-cli), and it has no impact on our nitro tools.

@t-bast t-bast merged commit 3a49f5d into ACINQ:master Jan 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants