pyRevit icon indicating copy to clipboard operation
pyRevit copied to clipboard

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

Open kirsch33 opened this issue 1 year ago • 2 comments

✈ 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

kirsch33 avatar Jul 01 '24 19:07 kirsch33

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.

sanzoghenzo avatar Jul 01 '24 20:07 sanzoghenzo

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.

kirsch33 avatar Jul 02 '24 12:07 kirsch33

This is actually causing me some issues now so I've chosen your first option above and created a PR, open to feedback:

https://github.com/pyrevitlabs/pyRevit/pull/2309

kirsch33 avatar Jul 09 '24 23:07 kirsch33

in the meantime, im trying to figure a workaround by completely suppressing all errors on a script level. i tried the following within a script that calls a pyRevit transaction:

logger = script.get_logger()
logger.set_quiet_mode()

but it doesnt seem to be working. any ideas @sanzoghenzo ?

kirsch33 avatar Jul 11 '24 16:07 kirsch33

in the meantime, im trying to figure a workaround by completely suppressing all errors on a script level. i tried the following within a script that calls a pyRevit transaction:

logger = script.get_logger()
logger.set_quiet_mode()

but it doesnt seem to be working. any ideas @sanzoghenzo ?

What does print(logger.get_level()) outputs? It should show something like CRITICAL, a log level that is never used in the codebase

sanzoghenzo avatar Jul 11 '24 17:07 sanzoghenzo

it outputs 50 which corresponds to CRITICAL and in the script if I try logger.error("test") it does not activate the logging window which is what I want.

but if something in the transaction wrapper errors, it will still display the error level message. maybe the transaction wrapper is using a different logger ?

kirsch33 avatar Jul 11 '24 17:07 kirsch33