TypeErrors raised by plugin hooks are sometimes silently ignored

[relikd]

Hi, it took a while to identify the culprit of this bug. And I honestly cannot imagine what might cause it.
But this simple plugin example shows what is going wrong:

from lektor.pluginsystem import Plugin

class TrashPlugin(Plugin):
    def on_before_build(self, builder, source, **extra):
        try:
            self.custom_attr
        except AttributeError:
            self.custom_attr = 'a'
            raise TypeError('FAIL')

Instead of showing the callstack with the TypeError, lektor somehow forgets the exception and continues happily.
The only hint that something gone wrong is this warning:

/lektor/pluginsystem.py:174: DeprecationWarning: The plugin "trash" function "on_before_build" does not accept extra_flags. It should be updated to accept **extra so that it will not break if new parameters are passed to it by newer versions of Lektor.
warnings.warn(

(which is extra frustrating because ... "c'mon, its there" 😆 )

[relikd]

Oh, and replacing TypeError with ValueError also works fine. Seems like only some exception types are ignored.
On the other hand, TypeError works fine, for example, if you are in an except IndexError: environment.

[dairiki]

You can see the logic that Lektor is using to detect whether the hook supports the extra_flags option in pluginsystem.py:

1. Lektor calls the hook with extra_flags added to the **kwargs.
2. If that throws a TypeError (which is what is thrown when callables are passed incompatible arguments) then Lektor tries calling the hook a second time without the extra_flags.
3. If the second try succeeds, Lektor assumes the problem was that the hook uses the "old API" and doesn't support extra_flags.

Your example hook throws TypeError on the first call, but not on subsequent calls, so it is fooling Lektor's heuristics.
It seems like a pretty specific edge case. Do you have a less-contrived example that motivates this?

---

I think this could be fixed by changing PluginSystem.emit to use inspect.signature and inspect.Signature.bind (rather than an actual call) to test whether the hook's signature accepts extra_flags. (Then Lektor can be sure that any TypeError is due to an incompatible signature.)
Something like¹:

munged_kwargs = {**kwargs, "extra_flags": process_extra_flags(self.extra_flags)}
signature = inspect.signature(handler)
try:
    signature.bind(**munged_kwargs)
except TypeError:
    munged_kwargs.pop("extra_flags")

rv[plugin.id] = handler(**munged_kwargs)
if "extra_flags" not in munged_kwargs:
    warnings.warn(f"The plugin {plugin.id!r} function {funcname!r} does not accept extra_flags. [...]")

Footnotes

1. Untested. PRs accepted. 😉 ↩

[relikd]

I see, that explains it.

Dont be fooled by the contrived example. I was struggling with this because it seemed random when it happens. The raise TypeError can be nested and hidden away somewhere in a state machine. For example, I/O operations that depend on other factors than the internal state of lektor.

In my case, I add an extra attribute to builder. That attribute is present on the second run because the first run did create it. So it seems the old builder with **extra is reused for the new build call without **extra. And this also means that whenever a plugin-build crashes it will try to run it a second time.