[Bug]: transaction displays errors even if log_errors set to false

kirsch33 commented 4 days ago

✈ Pre-Flight checks

[X] I don't have SentinelOne antivirus installed (see above for the solution)
[X] I have searched in the issues (open and closed) but couldn't find a similar issue
[X] I have searched in the pyRevit Forum for similar issues
[X] I already followed the installation troubleshooting guide thoroughly
[X] I am using the latest pyRevit Version

🐞 Describe the bug

i am using the pyRevit transaction wrapper and for certain tools I do not want end users to ever see error messages even if the transaction fails. i have set log_errors=False but sometimes the logger window still displays the error message. i tracked down the issue to maybe be in this code block:

https://github.com/pyrevitlabs/pyRevit/blob/f9fa0d296386d548e198324dd752fb7fb5cd7b58/pyrevitlib/pyrevit/revit/db/transaction.py#L80

https://github.com/pyrevitlabs/pyRevit/blob/f9fa0d296386d548e198324dd752fb7fb5cd7b58/pyrevitlib/pyrevit/revit/db/transaction.py#L138

it should check if log_errors is true before logging this error like it does a few lines up? Or is this by design and I'm mis-understanding the intention of log_errors?

⌨ Error/Debug Message

not applicable

♻️ To Reproduce

No response

⏲️ Expected behavior

No response

🖥️ Hardware and Software Setup (please complete the following information)

not applicable

Additional context

No response

sanzoghenzo commented 4 days ago

Hi @kirsch33, thanks for pointing this out. I've checked the history of that module and it seems it was always like that, maybe @eirannejad could shed some light on that.

But I agree with you, the name log_errors suggests that all the errors should/shouldn't be written.

We have some options here:

if this is a bug, add the self._logerror check before the lines you pointed
if this was an intended feature, add a log_commit_errors argument to the init and use that to suppress the logging, so that we keep retro-compatibility.

kirsch33 commented 3 days ago

hey @sanzoghenzo

i am fine with either option and dont mind to submit a PR to address but would like some direction from another maintainer before doing so.

pyrevitlabs / pyRevit