itamarst / eliot

Eliot: the logging system that tells you *why* it happened
https://eliot.readthedocs.io
Apache License 2.0
1.09k stars 65 forks source link

In case of _DestinationsSendError inside a start_action block, infinite recursion occurs #476

Closed alextatarinov closed 2 years ago

alextatarinov commented 2 years ago

If the Destination is broken regardless of the message, meaning it cannot log the "eliot:destination_failure" message, for instance (this is obviously, a simplification):

class BrokenDestination():
    def __call__(self, message):
        raise ValueError

And there is current action, the log_message will not use a "Logger that won't retry", contrary to the comment in the code, but the action's one that does retry, leading to infinite recursion.

Now, I am not actually sure what's the fix should be but will be happy to submit a pull request if the solution is proposed. One idea that comes to mind is to override action._logger if the current_action() is not None.

https://github.com/itamarst/eliot/blob/8609d380866491abe9e97eb814e4a04478b5893e/eliot/_output.py#L208-L222 https://github.com/itamarst/eliot/blob/8609d380866491abe9e97eb814e4a04478b5893e/eliot/_action.py#L932-L942

itamarst commented 2 years ago

Thanks for the bug report.

The solution I think is to add a try/except around the final line where it logs the message:

try:
    log_message(**msg)
except:
    # Our second attempt to log something failed, we're just going to have to give up.
    pass

I'd love a PR with that plus a test, if you have the time.

alextatarinov commented 2 years ago

PR: https://github.com/itamarst/eliot/pull/477