Skip to content

Bug/ap 25686 try catch rwe error msgs #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bernd-wiswedel wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Bug/ap 25686 try catch rwe error msgs #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a942523
AP-25686: Improve error message handling in Try-Catch block execution
bernd-wiswedel Mar 1, 2026
3425b65
AP-25686: NodeMessage to be part of NodeExecutionStatus (= better err…
bernd-wiswedel Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions org.knime.core/.settings/.api_filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,12 @@
</message_arguments>
</filter>
</resource>
<resource path="src/eclipse/org/knime/core/node/workflow/execresult/NodeContainerExecutionStatus.java" type="org.knime.core.node.workflow.execresult.NodeContainerExecutionStatus">
<filter comment="added as part of AP-25686 - interface is internal API" id="403804204">
<message_arguments>
<message_argument value="org.knime.core.node.workflow.execresult.NodeContainerExecutionStatus"/>
<message_argument value="getNodeMessage()"/>
</message_arguments>
</filter>
</resource>
</component>
7 changes: 3 additions & 4 deletions org.knime.core/src/eclipse/org/knime/core/node/workflow/NodeExecutionJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
import org.knime.core.node.port.PortObject;
import org.knime.core.node.port.PortType;
import org.knime.core.node.util.StringFormat;
import org.knime.core.node.workflow.execresult.NodeContainerExecutionResult;
import org.knime.core.node.workflow.execresult.NodeContainerExecutionStatus;
import org.knime.core.util.ThreadPool;

Expand Down Expand Up @@ -245,9 +244,9 @@ private static boolean checkForTryCatchScope(final SingleNodeContainer snc,
// failure inside an active try-catch:
// make node inactive and preserve error message(s)
if (snc.setInactive()) {
String errorMessage = (status instanceof NodeContainerExecutionResult)
? ((NodeContainerExecutionResult)status).getNodeMessage().getMessage() : status.toString();
snc.setNodeMessage(NodeMessage.newError("Execution failed in Try-Catch block: " + errorMessage));
final NodeMessage msg = status.getNodeMessage().prependMessage("Execution failed in Try-Catch block: ");
String errorMessage = msg.getMessage();
snc.setNodeMessage(msg);
// and store information such that the catch-node can report it
tcslc.setErrorToFlowObjectStack(snc.getName(), errorMessage, null, null,
snc.getOutgoingFlowObjectStack());
Expand Down
2 changes: 1 addition & 1 deletion org.knime.core/src/eclipse/org/knime/core/node/workflow/SubNodeContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1554,7 +1554,7 @@ public Boolean call() throws Exception {
.filter(m -> NodeMessage.Type.RESET != m.getMessageType()) //
.orElse(NodeMessage.newError("<reason unknown>"));
setNodeMessage(nodeMessage);
return NodeContainerExecutionStatus.newFailure(nodeMessage.getMessage());
return NodeContainerExecutionStatus.newFailure(nodeMessage);
}
return allExecuted ? NodeContainerExecutionStatus.SUCCESS : NodeContainerExecutionStatus.FAILURE;
} finally {
Expand Down
1 change: 1 addition & 0 deletions ...ore/src/eclipse/org/knime/core/node/workflow/execresult/NodeContainerExecutionResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ protected NodeContainerExecutionResult(final NodeContainerExecutionResult toCopy
/** Get a node message that was set during execution.
* @return The node message. */
@JsonProperty("nodeMessage")
@Override
public NodeMessage getNodeMessage() {
return m_message;
}
Comment on lines +89 to 92
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getNodeMessage() currently returns m_message directly, but m_message can be null (e.g., if not set yet or if deserialization loads older results without the field). Since NodeContainerExecutionStatus#getNodeMessage() is documented as non-null and callers now invoke methods on it (e.g., prependMessage(...)), please normalize null to NodeMessage.NONE (and consider doing the same in setMessage(...)).

Copilot uses AI. Check for mistakes.
Expand Down
35 changes: 32 additions & 3 deletions ...ore/src/eclipse/org/knime/core/node/workflow/execresult/NodeContainerExecutionStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
package org.knime.core.node.workflow.execresult;

import org.knime.core.node.workflow.NodeID;
import org.knime.core.node.workflow.NodeMessage;
import org.knime.core.node.workflow.SingleNodeContainer;

/**
Expand All @@ -74,6 +75,12 @@
*/
public boolean isSuccess();

/** The node's message, corresponding to the node's execution status.
* @return the message, not null.
* @since 5.12
*/
NodeMessage getNodeMessage();

/** Query the execution status for a child given its
* {@linkplain NodeID#getIndex() node id suffix}. If the child is unknown,
* the implementation should return {@link #FAILURE}.
Expand All @@ -85,8 +92,19 @@
/** Convenience shortcut to create failure with no children but custom error message.
* @param message the message
* @return a new failure with a custom message.
* @since 3.0 */
static public NodeContainerExecutionStatus newFailure(final String message) {
* @since 3.0
* @deprecated Use {@link #newFailure(NodeMessage)} instead.
*/
@Deprecated(since = "5.12", forRemoval = true)
static NodeContainerExecutionStatus newFailure(final String message) {

Check warning on line 99 in org.knime.core/src/eclipse/org/knime/core/node/workflow/execresult/NodeContainerExecutionStatus.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Do not forget to remove this deprecated code someday. 


See more on https://sonarcloud.io/project/issues?id=KNIME_knime-core&issues=AZyrIj8VH8mnusrfcF6V&open=AZyrIj8VH8mnusrfcF6V&pullRequest=73
return newFailure(new NodeMessage(NodeMessage.Type.ERROR, message));
}
Comment on lines +98 to +101
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newFailure(String) overload is now deprecated/forRemoval, but this interface still uses it internally (e.g., to initialize the FAILURE constant). To avoid introducing new deprecation warnings (and future breakage once removed), update internal call sites to use newFailure(NodeMessage) directly.

Copilot uses AI. Check for mistakes.

/** Convenience shortcut to create failure with no children but custom error message.
* @param message the message
* @return a new failure with a custom message.
* @since 5.12 */
static NodeContainerExecutionStatus newFailure(final NodeMessage message) {
return new NodeContainerExecutionStatus() {

/**
Expand All @@ -98,6 +116,12 @@
return this;
}


@Override
public NodeMessage getNodeMessage() {
return message;
}

/** @return false */
@Override
public boolean isSuccess() {
Expand All @@ -107,13 +131,13 @@
/** {@inheritDoc} */
@Override
public String toString() {
return message;
return message.getMessage();
Comment on lines 107 to +134
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newFailure(NodeMessage message) assumes message is non-null (used in getNodeMessage() and toString()), but there is no null check. Given the interface contract that getNodeMessage() returns non-null, please enforce non-null here (e.g., requireNonNull / CheckUtils) or normalize null to NodeMessage.NONE (or an ERROR placeholder) to avoid potential NPEs.

Copilot uses AI. Check for mistakes.
}
};
}

/** Represents a failed execution. */
public static final NodeContainerExecutionStatus FAILURE = newFailure("Failure execution status");

Check warning on line 140 in org.knime.core/src/eclipse/org/knime/core/node/workflow/execresult/NodeContainerExecutionStatus.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this call to a deprecated method, it has been marked for removal. 


See more on https://sonarcloud.io/project/issues?id=KNIME_knime-core&issues=AZyrIj8VH8mnusrfcF6W&open=AZyrIj8VH8mnusrfcF6W&pullRequest=73

/** Represents a successful execution. */
public static final NodeContainerExecutionStatus SUCCESS =
Expand All @@ -129,6 +153,11 @@
return SUCCESS;
}

@Override
public NodeMessage getNodeMessage() {
return NodeMessage.NONE;
}

/** @return true */
@Override
public boolean isSuccess() {
Expand Down