Enhance Error Message for Missing Core Lightning Configuration File #211
Conversation
✅ Deploy Preview for coffee-docs canceled.
|
tareknaser
force-pushed
the
better-error
branch
from
6ed1f5d to
5e20bf5
Compare
tareknaser
force-pushed
the
better-error
branch
from
5e20bf5 to
497b721
Compare
The error message displayed in case of a missing Core Lightning configuration file has been enhanced to provide users with more friendly information. Signed-off-by: Tarek <[email protected]>
tareknaser
force-pushed
the
better-error
branch
from
497b721 to
27a285c
Compare
| let root = self.config.cln_root.clone(); | ||
| let root = root.ok_or_else(|| error!("cln root path not found."))?; | ||
| let rpc = Client::new(format!("{root}/{}/lightning-rpc", self.config.network)); | ||
| self.rpc = Some(rpc); | ||
| let path = self.config.cln_config_path.clone().unwrap(); | ||
| let path = self.config.cln_config_path.clone(); | ||
| let path = path.ok_or_else(|| error!("cln config path not found."))?; |
There was a problem hiding this comment.
what about the path is Some but do not exist?
There was a problem hiding this comment.
The next line is
let mut file = CLNConf::new(path.clone(), true);where we set create_if_missing to true
This is what happens when the user first runs coffee
There was a problem hiding this comment.
Mh I do not think this is right, if the path do not exist this code do not make sense because CLNConf::new(path.clone(), true); is not able to find the path where to create the file.
Please test the user case that I described #209
There was a problem hiding this comment.
You are right.
This line will handle the case that the root path exists but the configuration file doesn't.
However, it will fail if the root path doesn't exist.
There was a problem hiding this comment.
Added another check for this case.
There was a problem hiding this comment.
This line will handle the case that the root path exists but the configuration file doesn't.
This is done already by CLNConf
There was a problem hiding this comment.
I am not sure CLNConf handles this case.
Could you give it another try?
To reproduce:
1- delete ~/.lightning/bitcoin directory
2- try running coffee commands (eg. coffee remote list)
You will get the same error mentioned in the original issue
What CLNConf handles is the case when ~/.lightning/bitcoin/config is missing but ~/.lightning/bitcoin exists.
There was a problem hiding this comment.
I am not sure CLNConf handles this case.
yes is does, you need to do CLNConf::new(path.clone(), false); if the file do not exist it return an error
What CLNConf handles is the case when ~/.lightning/bitcoin/config is missing but ~/.lightning/bitcoin exists.
Correct, you should check if the path exist
This commit addresses the scenario where the lightning root folder is not found. Previously, Coffee would provide a generic error message (path not found), causing confusion.
With this update, the error message has been refined to provide more clarity. Now, if the lightning root folder is not found, the error message specifically states: The path {lightning_path} does not exist.
Signed-off-by: Tarek <[email protected]>
| // otherwise we return an error. | ||
|
|
||
| // We can unwrap here because we are sure that the path ends with /config | ||
| let lightning_path = path.strip_suffix("/config").unwrap(); |
There was a problem hiding this comment.
If you need to strip this, this means that you are checking in the wrong place.
The check need to be done before we build the path with the /config
| // We need to check that the root folder (eg. ~/.lightning/bitcoin) exists | ||
| // otherwise we return an error. | ||
|
|
||
| // We can unwrap here because we are sure that the path ends with /config |
There was a problem hiding this comment.
// SAFETY: We can unwrap here because we are sure that the path ends with /config
|
I think this code should be simpler, and I try to see if this is true #220 |
|
Closing this pull request and going ahead with #220. It has the same fix but is simpler. |
The error message displayed in case of a missing Core Lightning configuration file has been enhanced to provide users with more friendly and actionable information.
Now, when coffee fails to find the configuration file at the specified path, it advises users to resolve the issue by running the
coffee setup <cln_path>command, helping them quickly address the problem and proceed smoothly.Fixes #209