-
Notifications
You must be signed in to change notification settings - Fork 69
Description
Using @log_call fails and generates errors completely unrelated to the actual execution when any arguments or return types are not JSON-serialisable. I imagine the same is also the case for any other logging function, but @log_call where I ran into it, since it's much more probable that arbitrary types will be logged in that situation.
Small repro case:
import subprocess
import eliot
@eliot.log_call
def foo(x):
return x
eliot.to_file(open("/tmp/the-log.json", "wb"))
foo(subprocess.Popen("date"))Results:
$ eliot-tree /tmp/the-log.json
3a6afc51-3f52-4e3f-9bf1-9de3943ab9c4
└── eliot:destination_failure/1 2025-04-13 16:03:33Z
├── exception: builtins.TypeError
├── message: {"'x'": "<Popen: returncode: None args: 'date'>", "'action_status'": "'started'", "'timestamp'": '17…
└── reason: Type is not JSON serializable: Popen
1857c350-f0d4-4c84-8063-8c8426f24b08
└── eliot:destination_failure/1 2025-04-13 16:03:33Z
├── exception: builtins.TypeError
├── message: {"'result'": "<Popen: returncode: None args: 'date'>", "'action_status'": "'succeeded'", "'timestamp…
└── reason: Type is not JSON serializable: Popen
This is, IMHO, unacceptable for a general-purpose logging library, especially since there's not a word anywhere in the documentation about the requirement for items being logged to be fully JSON-serialisable. If a value doesn't have a predefined serialiser, it should just fall back to repr(), rather than logging errors unrelated to the code being executed and completely dropping the actual logs. Slightly lower-fidelity logs that never fail are far, far superior to no logs at all and confusing errors.