Prevent bootlooping due to a failed plugin load.

[aduane]

We already skip plugins that fail to load due to an ImportError, this change increases the types of errors that would skip a plugin considerably. It is extremely important to prevent the process from crashing on load, as circus will immediately attempt to resurrect it, causing a bootloop.

[beaugunderson]

hmm why not just pass the whole exception to log.error?

[aduane]

It was a design goal to accomplish this in the most silly way possible. (I will fix)

[aduane]

Well wait, we do that above in the string interpolation. It doesn't show the whole stack. I would like the log output to show the line number of the offending class.

[beaugunderson]

you would still call format_exception, but just pass the output to log.error directly rather than every line... let me test real quick if it actually does what I think

[beaugunderson]

ok I stand corrected, how about this:

log.error('\n'.join(traceback.format_exception(err))

[aduane]

This might just be a preference thing, but yours messes with the indent due to the single log timestamp vs one per line.

yours:

ERROR 2024-07-16 10:54:10,033 Error importing module 'search:search.protocols.condition:Protocol': EnumTypeWrapper.Name() takes 2 positional arguments but 3 were given
ERROR 2024-07-16 10:54:10,034 Traceback (most recent call last):

  File "/Users/andrew/src/canvas-plugins/plugin_runner/plugin_runner.py", line 198, in load_or_reload_plugin
    result = sandbox_from_module_name(protocol_module)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/Users/andrew/src/canvas-plugins/plugin_runner/plugin_runner.py", line 139, in sandbox_from_module_name
    return sandbox.execute()
           ^^^^^^^^^^^^^^^^^

  File "/Users/andrew/src/canvas-plugins/plugin_runner/sandbox.py", line 267, in execute
    exec(self.compile_result.code, self.scope)

  File "<string>", line 1, in <module>

  File "<string>", line 14, in Protocol

TypeError: EnumTypeWrapper.Name() takes 2 positional arguments but 3 were given

mine:

ERROR 2024-07-16 10:25:40,695 Error importing module 'search:search.protocols.condition:Protocol': EnumTypeWrapper.Name() takes 2 positional arguments but 3 were given
ERROR 2024-07-16 10:25:40,696 Traceback (most recent call last):

ERROR 2024-07-16 10:25:40,696   File "/Users/andrew/src/canvas-plugins/plugin_runner/plugin_runner.py", line 198, in load_or_reload_plugin
    result = sandbox_from_module_name(protocol_module)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ERROR 2024-07-16 10:25:40,696   File "/Users/andrew/src/canvas-plugins/plugin_runner/plugin_runner.py", line 139, in sandbox_from_module_name
    return sandbox.execute()
           ^^^^^^^^^^^^^^^^^

ERROR 2024-07-16 10:25:40,696   File "/Users/andrew/src/canvas-plugins/plugin_runner/sandbox.py", line 267, in execute
    exec(self.compile_result.code, self.scope)

ERROR 2024-07-16 10:25:40,696   File "<string>", line 1, in <module>

ERROR 2024-07-16 10:25:40,696   File "<string>", line 14, in Protocol

ERROR 2024-07-16 10:25:40,696 TypeError: EnumTypeWrapper.Name() takes 2 positional arguments but 3 were given

Which do you prefer? I don't feel strongly.

[beaugunderson]

i have a very slight preference for mine because it's easier to read i think... on the flip side it would be impossible to search for in kibana (whereas with your method you could search for all lines that start with ERROR) so maybe that's the more important criteria

[aduane]

You know, now that I think about it a bit more, having the individual error lines will be more important when the hostname is also included on the line so we can separate out overlapping logs in multi-container deployments.

[aduane]

So if we keep it as-is, I'll just need a ✅

[csande]

It doesn't help here if you want ERROR on every line, but I just wanted to note that log.exception('error message') formats and logs the traceback automatically without a need to call format_exception.