Add optional debug logging #95
Conversation
| return this._logger; | ||
| } | ||
|
|
||
| abstract execute(input: InputType): Promise<O.Option<OutputType>>; |
There was a problem hiding this comment.
Sinks would then return Promise<O.Some&l
8000
t;void>>? Sounds good to me, unless we want to stack errors (like a stack trace) - then we should bubble up the error.
There was a problem hiding this comment.
Such an error bubbling mechanism would indeed make sense for composite blocks in the future. But I think that event the previous implementation would have to be further adjusted in order to support stack traces. A possibility I can think of, is to have some kind of execution context in block executors, to which errors are reported. The execution context then would know the "stacks" above.
My proposal is to keep the refactoring and at a later point put more thoughts into error management and implement a new system.
There was a problem hiding this comment.
I am not sure I understand the reasoning to return None from blocks that fail. That seems semantically counterintuitive to me. We can still return an error result after the whole execution and just not immediately exit with an error. That way we are still able to log multiple things but we would not continue executing a pipeline that has clearly failed. Also the difference between an error result and an empty result would be more clear.
I might be missing something there so I am happy to be convinced otherwise though 😅 .
| @@ -30,6 +32,7 @@ export function createBlockExecutor( | |||
| const blockExecutorInstance = new blockExecutor(); | |||
| blockExecutorInstance.block = block; | |||
| blockExecutorInstance.runtimeParameters = runtimeParameters; | |||
| blockExecutorInstance.logger = logger; | |||
There was a problem hiding this comment.
At some point, the logger should know what it is logging for. So either the block type should be set here or in the logX methods in the actual executor parent class. That way we can build nicer log messages with the addition of the block type.
Part of #86
Introduces a debug flag (
-d/--debug) in the CLI which enables debug logging. This means that logs with severityinfoare printed during execution (this is not the case by default). Note that info logging is still missing in many places, but this is considered outside the scope of this PR.This PR also contains some internal refactorings:
Loggerinterface and its implementationDefaultLoggerDefaultLoggeris used in the interpreter to perform the actual loggingThe error handling in block executors now use a different pattern:Previously, in case of errors, an error object was returned as execution resultThe error then was printed outside the executorThis was also the case in other places (e.g. when processing runtime parameters)The new way to report errors is to log them directly in the block executorThat way, we have more freedom in logging errors (e.g. multiple errors with different severities) and they are printed to the console immediatelyFor reporting the result of a block execution,Optionfromfp-tsis usedI.e.O.some(resultingData)for a successful execution and elseO.none