[pdm-devel] [RFC PATCH datacenter-manager] server: api: resources: use root cause for errors for remote fetching

[pdm-devel] [RFC PATCH datacenter-manager] server: api: resources: use root cause for errors for remote fetching

[Dominik Csapak]

when we can't reach a remote for any reason, we want to return the error
as a string here over the API. Since most errors that can occur here are
client/network related (wrong credentials, no route to host, timeout,
etc.) converting this error directly to a string gives us
errors like:

`client error (Connect)`

which is not really helpful most of the time.
Instead if we use the `root_cause()`, we get the most underlying error

e.g.

`error connecting to https://0.0.0.0:8006/ - tcp connect error: No route to host (os error 113)`

which is much more helpful.

We could also think about printing the whole error chain, but in my test
cases here this was not more helpful, e.g. i got two times the above
`client error (Connect)` and once the root cause from above.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 server/src/api/resources.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 98c4dea..cca8bcb 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -245,7 +245,7 @@ pub(crate) async fn get_resources_impl(
         let handle = tokio::spawn(async move {
             let (mut resources, error) = match get_resources_for_remote(remote, max_age).await {
                 Ok(resources) => (resources, None),
-                Err(error) => (Vec::new(), Some(error.to_string())),
+                Err(error) => (Vec::new(), Some(error.root_cause().to_string())),
             };
 
             if remotes_only {
-- 
2.47.2



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

Re: [pdm-devel] [RFC PATCH datacenter-manager] server: api: resources: use root cause for errors for remote fetching

[Wolfgang Bumiller]

On Tue, Sep 09, 2025 at 10:25:51AM +0200, Dominik Csapak wrote:
> when we can't reach a remote for any reason, we want to return the error
> as a string here over the API. Since most errors that can occur here are
> client/network related (wrong credentials, no route to host, timeout,
> etc.) converting this error directly to a string gives us
> errors like:
> 
> `client error (Connect)`
> 
> which is not really helpful most of the time.
> Instead if we use the `root_cause()`, we get the most underlying error
> 
> e.g.
> 
> `error connecting to https://0.0.0.0:8006/ - tcp connect error: No route to host (os error 113)`
> 
> which is much more helpful.
> 
> We could also think about printing the whole error chain, but in my test
> cases here this was not more helpful, e.g. i got two times the above
> `client error (Connect)` and once the root cause from above.

Curious, because from a quick grep, `client error (*)` comes from hyper.
Otherwise I'd say it may have been one our own error types or
one-too-many same `.context()` calls.

I'm still not sure this is a good idea though, as we may still lose some
useful contest. I wish anyhow had an easy method to just shave off a
single layer.

What if we do this, but first throw in a:

    tracing::debug!("{error:?}");

just so we still have the option to see everything if we need to?

> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  server/src/api/resources.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index 98c4dea..cca8bcb 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -245,7 +245,7 @@ pub(crate) async fn get_resources_impl(
>          let handle = tokio::spawn(async move {
>              let (mut resources, error) = match get_resources_for_remote(remote, max_age).await {
>                  Ok(resources) => (resources, None),
> -                Err(error) => (Vec::new(), Some(error.to_string())),
> +                Err(error) => (Vec::new(), Some(error.root_cause().to_string())),
>              };
>  
>              if remotes_only {
> -- 
> 2.47.2


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

Re: [pdm-devel] [RFC PATCH datacenter-manager] server: api: resources: use root cause for errors for remote fetching

[Dominik Csapak]

On 9/10/25 10:15 AM, Wolfgang Bumiller wrote:
> On Tue, Sep 09, 2025 at 10:25:51AM +0200, Dominik Csapak wrote:
>> when we can't reach a remote for any reason, we want to return the error
>> as a string here over the API. Since most errors that can occur here are
>> client/network related (wrong credentials, no route to host, timeout,
>> etc.) converting this error directly to a string gives us
>> errors like:
>>
>> `client error (Connect)`
>>
>> which is not really helpful most of the time.
>> Instead if we use the `root_cause()`, we get the most underlying error
>>
>> e.g.
>>
>> `error connecting to https://0.0.0.0:8006/ - tcp connect error: No route to host (os error 113)`
>>
>> which is much more helpful.
>>
>> We could also think about printing the whole error chain, but in my test
>> cases here this was not more helpful, e.g. i got two times the above
>> `client error (Connect)` and once the root cause from above.
> 
> Curious, because from a quick grep, `client error (*)` comes from hyper.
> Otherwise I'd say it may have been one our own error types or
> one-too-many same `.context()` calls.
> 
> I'm still not sure this is a good idea though, as we may still lose some
> useful contest. I wish anyhow had an easy method to just shave off a
> single layer.
> 
> What if we do this, but first throw in a:
> 
>      tracing::debug!("{error:?}");
> 
> just so we still have the option to see everything if we need to?

just to clarify: you want me to use root_cause and additionally log the 
error into the debug tracing? (not returning the whole `chain()` here)

> 
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   server/src/api/resources.rs | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
>> index 98c4dea..cca8bcb 100644
>> --- a/server/src/api/resources.rs
>> +++ b/server/src/api/resources.rs
>> @@ -245,7 +245,7 @@ pub(crate) async fn get_resources_impl(
>>           let handle = tokio::spawn(async move {
>>               let (mut resources, error) = match get_resources_for_remote(remote, max_age).await {
>>                   Ok(resources) => (resources, None),
>> -                Err(error) => (Vec::new(), Some(error.to_string())),
>> +                Err(error) => (Vec::new(), Some(error.root_cause().to_string())),
>>               };
>>   
>>               if remotes_only {
>> -- 
>> 2.47.2



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

Re: [pdm-devel] [RFC PATCH datacenter-manager] server: api: resources: use root cause for errors for remote fetching

[Wolfgang Bumiller]

On Wed, Sep 10, 2025 at 10:31:52AM +0200, Dominik Csapak wrote:
> 
> 
> On 9/10/25 10:15 AM, Wolfgang Bumiller wrote:
> > On Tue, Sep 09, 2025 at 10:25:51AM +0200, Dominik Csapak wrote:
> > > when we can't reach a remote for any reason, we want to return the error
> > > as a string here over the API. Since most errors that can occur here are
> > > client/network related (wrong credentials, no route to host, timeout,
> > > etc.) converting this error directly to a string gives us
> > > errors like:
> > > 
> > > `client error (Connect)`
> > > 
> > > which is not really helpful most of the time.
> > > Instead if we use the `root_cause()`, we get the most underlying error
> > > 
> > > e.g.
> > > 
> > > `error connecting to https://0.0.0.0:8006/ - tcp connect error: No route to host (os error 113)`
> > > 
> > > which is much more helpful.
> > > 
> > > We could also think about printing the whole error chain, but in my test
> > > cases here this was not more helpful, e.g. i got two times the above
> > > `client error (Connect)` and once the root cause from above.
> > 
> > Curious, because from a quick grep, `client error (*)` comes from hyper.
> > Otherwise I'd say it may have been one our own error types or
> > one-too-many same `.context()` calls.
> > 
> > I'm still not sure this is a good idea though, as we may still lose some
> > useful contest. I wish anyhow had an easy method to just shave off a
> > single layer.
> > 
> > What if we do this, but first throw in a:
> > 
> >      tracing::debug!("{error:?}");
> > 
> > just so we still have the option to see everything if we need to?
> 
> just to clarify: you want me to use root_cause and additionally log the
> error into the debug tracing? (not returning the whole `chain()` here)

Yes. Log the complete error at debug-level but only return the root
cause.


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

[pdm-devel] applied: [RFC PATCH datacenter-manager] server: api: resources: use root cause for errors for remote fetching

[Thomas Lamprecht]

On Tue, 09 Sep 2025 10:25:51 +0200, Dominik Csapak wrote:
> when we can't reach a remote for any reason, we want to return the error
> as a string here over the API. Since most errors that can occur here are
> client/network related (wrong credentials, no route to host, timeout,
> etc.) converting this error directly to a string gives us
> errors like:
> 
> `client error (Connect)`
> 
> [...]

Applied, thanks! Added logging the full context as tracing::debug message like
Wolfgang suggested in a follow up.

[1/1] server: api: resources: use root cause for errors for remote fetching
      commit: 56a80782168ce5cc7f5da6eaafe533d9f791ef38


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel