Intentional vs. Unintentional exceptions

[PaulSchweizer]

This is a general question on error handling in pyblish.

Currently (as far as I understand), any error raised in a Plugin is being caught and added to the report data.

The problem we are experiencing is that we would like to distinguish between intentional and unintentional errors.

• Intentional errors are those that we raise in order to mark a Plugin as failed
• Unintentional errors are those that happen due to programming errors

Since all errors are caught it is:

• Hard to distinguish whether the plugin failed deliberately or due to a mistake in the program
• Hard to debug as there is no stack trace (by default at least)

Please consider this code example to illustrate the point:

import pyblish.api
import pyblish.error
import pyblish.util

class SomeValidator(pyblish.api.Validator):

    def process(self, instance):

        # These errors are deliberately added and used to mark a failing validation
        assert False, "This is a deliberate error"  # Assertions seem to be used a lot in pyblish which is why I include them there
        raise pyblish.error.PyblishError("This is a deliberate error")  # And all it's sub-classes

        # This is an unintentional programming error (IndexError) and should NOT be caught
        # by pyblish, it should just fail the usual way.
        empty_list = []
        print(empty_list[0])

if __name__ == "__main__":
    pyblish.api.register_plugin(TestingValidator)
    pyblish.api.discover()
    context = pyblish.api.Context()
    context.create_instance(name="MyInstance")
    pyblish.util.publish(context)

My immediate suggestion would be to distinguish between intentional und unintentional errors and only catch the intentional ones, but probably I am missing a design decision? I'm aware that this change would break code that is using "regular" Exceptions as intentional errors.

[tokejepsen]

Interesting suggestion!

Have you encountered issues with the currently design in production? The idea behind the design is that any unintentional errors should be accounted for in the plugin, but similar to test driven development you cant anticipate everything. What usually happens is that these unintentional errors are flagged during production by the crew, mostly when they can't get past validation, and fixed in the plugin.

[davidlatwe]

Hi @PaulSchweizer :) What you were suggesting is to stop publish process immediately if unintentional error raised, am I correct ? If we go this way, then we need #352 be implemented as well, I think. If not, what are you excepting after unintentional error raised ?

[PaulSchweizer]

@davidlatwe , yes I would expect the python process to exit by raising the "unintentional" exception as such an exception is not part of pyblish, it's just an exception that happened. And yes, I think implementing #352 would provide a good safety net for these cases.

@tokejepsen the main issue for us is that since all errors are caught and they don't provide a stack trace that we can look at during development. Also artists can't copy paste anyting useful into a ticket. This means that we have a harder time debugging those unintentional errors. "IndexError: list index out of range" is just not very helpful. It also caused some confusion when checks failed because of an "unintentional" error and people were searching their scenes for the cause of the failed check. Some of those issues are of course down to a lack of experience with the toolset but I think that treating unintentional errors as regular exceptions would alleviate those issues immediately.

[tokejepsen]

yes I would expect the python process to exit by raising the "unintentional" exception

So you want to have the whole publish process exit with the unintentional error raising for a stacktrace?

Also artists can't copy paste anyting useful into a ticket.

What UI are you using? Both pyblish-qml and pyblish-lite are lacking in this area. What I have done in the past is to have a "Report" plugin, where artists can copy the errors to the clipboard to send.

[PaulSchweizer]

So you want to have the whole publish process exit with the unintentional error raising for a stacktrace?

Yes

What UI are you using?

We are currently using our own fork of the pyblish-lite UI. We added farm submission support and some more informational fields for the user. We could do something similar to what you describe indeed and classfiy the exceptions ourselves in our own fork. We are however also using pyblish without the UI so having this behaviour directly in pyblish would be better for us.

[mottosso]

How about a post-validator, that checks to see whether there are any unintential errors?

class MyPostValidator(api.ContextPlugin):
  order = api.Validator + 0.49

  def process(self, context):
    intentional_errors = [AssertionError, PyblishError]

    for result in context.data["results"]:
      if not result["error"]:
        continue

      assert type(result["error"]) in intentional_errors, (
        "Hol'up a minit, %s wasn't accounted for" % result["error"]
      )

[PaulSchweizer]

Nice solution as it would not break anything but it would still not give me a stack trace though.

[BigRoy]

Couldn't a custom registered test also make it "directly stop"? That would allow you to capture your own cases of when continuing to process should continue.

The stack trace is a separate issue where pyblish itself should somehow store the stack trace with the error so it can be used for the output report. I don't think that's currently stored in a way where the full stack trace can be reported.

[tokejepsen]

There is an effort to get better stacktracing here; https://github.com/pyblish/pyblish-base/pull/347

[mottosso]

I was certain the error included a stack trace, but it doesn't appear to. :O

from pyblish import api, util

class TestValidator(api.InstancePlugin):
    order = api.ValidatorOrder

    def process(self, instance):
        empty_list = []
        print(empty_list[0])

if __name__ == "__main__":
    api.register_plugin(TestValidator)
    api.discover()

    context = api.Context()
    context.create_instance(name="MyInstance")
    util.publish(context)

    for result in context.data["results"]:
        error = result["error"]

        if not error:
            continue

        print(error.args)
        print(error.message)
        print(error.traceback)

# ('list index out of range',)                                                                                 
# list index out of range                                                                                      
# ('.\\temp.py', 9, 'process', 'print(empty_list[0])')  

These are what's available for an error, and traceback is just filename, line number and message rolled into one.

With #347 (or a successor to it, rather), I would expect another member:

     print(error.stacktrace)

# Traceback (most recent call last):                                                                           
#   File ".\temp.py", line 12, in process                                                                      
#     print(empty_list[0])                                                                                     
# IndexError: list index out of range                                                                          
#                                                                                                              
# list index out of range

Which I think should account for everyone's needs.

[PaulSchweizer]

I think once we get the stacktrace in the results, we can work around this on our side since we use our own fork of the lite ui and can enforce our own rules as to how we interpret the different exceptions.

[PaulSchweizer]

Thanks for merging https://github.com/pyblish/pyblish-base/pull/356 My immediate problems are now solved and you can close this issue.

That being said, I am still a bit sceptical about the concept of capturing all exceptions, as it feels a bit like hijacking the regular python exception management. But again, the PR solved my issues and we can work with it the way it is right now