Make LogContextPipe not throw when it has an error during execution #6172
Conversation
| } catch (ParameterException e) { | ||
| throw new PipeRunException(this, "exception extracting parameters", e); | ||
| } catch (Exception e) { | ||
| log.debug("Exception getting parameter values. Ignoring.", e); |
There was a problem hiding this comment.
Could this be changed to a warn statement?
If you see this a lot in production, something might be wrongly/sub-optimal configured..
There was a problem hiding this comment.
I was doubting because some messages we did make warnings, turned out to be excessive without pointing to any kind of misconfiguration.
But in this case you're probably right.
| } catch (ParameterException e) { | ||
| throw new PipeRunException(this, "exception extracting parameters", e); | ||
| } catch (Exception e) { | ||
| log.warn("Exception getting parameter values. Ignoring.", e); |
There was a problem hiding this comment.
Just wondering if this is enough info for the end user (integration specialist) to understand/find the configuration which is wrong. @Laurens-makel what is your opinion?
There was a problem hiding this comment.
I think the short answer here would be no. And than I would also like to propose an option to use the exception forward by default unless an additional property or setting has been set.
There was a problem hiding this comment.
Depends on what is in e... Does it mention which parameter caused the exception?
There was a problem hiding this comment.
Depends on what is in
e... Does it mention which parameter caused the exception?
No, it only mentions the stacktrace, which isn't helping.
| if (!message.isRepeatable()) { | ||
| message.preserve(); | ||
| } |
There was a problem hiding this comment.
Dit is onderdeel van de ParameterValueList en hoeft hier niet.
| } catch (ParameterException e) { | ||
| throw new PipeRunException(this, "exception extracting parameters", e); | ||
| } catch (Exception e) { | ||
| log.warn("Exception getting parameter values. Ignoring.", e); |
There was a problem hiding this comment.
I think the short answer here would be no. And than I would also like to propose an option to use the exception forward by default unless an additional property or setting has been set.
Add `parameterName` to the ParameterException so that we can start reporting explicitly on the parameter that caused an error.
tnleeuw
requested review from
jkosternl,
nielsm5 and
Laurens-makel
and removed request for
Yale96
No description provided.