[pve-devel] [PATCH proxmox-firewall] firewall: show full error if rule apply fails

[pve-devel] [PATCH proxmox-firewall] firewall: show full error if rule apply fails

[Christoph Heiss]

Currently, this will only log something like

  error updating firewall rules: cannot execute nftables commands

which, by itself, is pretty unhelpful. `anyhow` errors have context, so
show that too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-firewall/src/bin/proxmox-firewall.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 4732e51..5cd0c70 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -101,7 +101,7 @@ fn main() -> Result<(), std::io::Error> {
         let start = Instant::now();
 
         if let Err(error) = handle_firewall() {
-            log::error!("error updating firewall rules: {error}");
+            log::error!("error updating firewall rules: {error:#}");
         }
 
         let duration = start.elapsed();
-- 
2.49.0



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

Re: [pve-devel] [PATCH proxmox-firewall] firewall: show full error if rule apply fails

[Stefan Hanreich]

On 6/30/25 13:23, Christoph Heiss wrote:
> Currently, this will only log something like
> 
>   error updating firewall rules: cannot execute nftables commands
> 
> which, by itself, is pretty unhelpful. `anyhow` errors have context, so
> show that too.

How much additional lines in the log would that be? Since the daemon
runs every 5s this could grow large quite quickly. I already made some
effort of improving the debug experience by introducing new subcommands
[1], similar to pve-firewall. So potentially that is sufficient?
Otherwise we could also make this dependent on the log level?

[1]
https://lore.proxmox.com/pve-devel/20250527135800.190084-3-s.hanreich@proxmox.com/

> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  proxmox-firewall/src/bin/proxmox-firewall.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
> index 4732e51..5cd0c70 100644
> --- a/proxmox-firewall/src/bin/proxmox-firewall.rs
> +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
> @@ -101,7 +101,7 @@ fn main() -> Result<(), std::io::Error> {
>          let start = Instant::now();
>  
>          if let Err(error) = handle_firewall() {
> -            log::error!("error updating firewall rules: {error}");
> +            log::error!("error updating firewall rules: {error:#}");
>          }
>  
>          let duration = start.elapsed();



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

Re: [pve-devel] [PATCH proxmox-firewall] firewall: show full error if rule apply fails

[Christoph Heiss]

On Wed Jul 2, 2025 at 3:42 PM CEST, Stefan Hanreich wrote:
> On 6/30/25 13:23, Christoph Heiss wrote:
>> Currently, this will only log something like
>>
>>   error updating firewall rules: cannot execute nftables commands
>>
>> which, by itself, is pretty unhelpful. `anyhow` errors have context, so
>> show that too.
>
> How much additional lines in the log would that be?

This should not result in any extra lines, as the {:#} specifier puts
everything on one line [0]. As long as the individual messages/contexts
don't have newlines in them (which they shouldn't), of course.

[0] https://docs.rs/anyhow/latest/anyhow/struct.Error.html



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

Re: [pve-devel] [PATCH proxmox-firewall] firewall: show full error if rule apply fails

[Stefan Hanreich]

On 7/2/25 15:53, Christoph Heiss wrote:
> On Wed Jul 2, 2025 at 3:42 PM CEST, Stefan Hanreich wrote:
>> On 6/30/25 13:23, Christoph Heiss wrote:
>>> Currently, this will only log something like
>>>
>>>   error updating firewall rules: cannot execute nftables commands
>>>
>>> which, by itself, is pretty unhelpful. `anyhow` errors have context, so
>>> show that too.
>>
>> How much additional lines in the log would that be?
> 
> This should not result in any extra lines, as the {:#} specifier puts
> everything on one line [0]. As long as the individual messages/contexts
> don't have newlines in them (which they shouldn't), of course.

Ah, sorry, I was confusing it with {:?}


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

Re: [pve-devel] [PATCH proxmox-firewall] firewall: show full error if rule apply fails

[Thomas Lamprecht]

On Mon, 30 Jun 2025 13:23:32 +0200, Christoph Heiss wrote:
> Currently, this will only log something like
> 
>   error updating firewall rules: cannot execute nftables commands
> 
> which, by itself, is pretty unhelpful. `anyhow` errors have context, so
> show that too.
> 
> [...]

Applied, thanks!

[1/1] firewall: show full error if rule apply fails
      commit: 1617b6c993ca3ccaec52661951cda5d69ca7a37e



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