[Improvement-18675][API] Return TASK_DEFINE_NOT_EXIST instead of NPE when taskRelation references missing task definitions

[wank125]

Was this PR generated or assisted by AI?

YES — the fix, the unit test and this description were drafted with AI assistance (Claude-based coding agent); the bug itself was discovered while driving the DolphinScheduler 3.2.2 REST API from a data-sync pipeline. The change was reviewed, compiled and tested locally by the submitter.

Purpose of the pull request

Fixes #18675

When taskRelationJson references a task code that has no matching entry in taskDefinitionJson, ProcessServiceImpl#transformTask throws a NullPointerException:

taskNode.setPreTasks(JSONUtils.toJsonString(code.getValue().stream().map(taskDefinitionLogMap::get)
        .map(TaskDefinition::getCode).collect(Collectors.toList())));

The NPE is caught by the generic catch (Exception e) in WorkflowDefinitionServiceImpl#generateTaskRelationList and returned to the client as a generic 10001 REQUEST_PARAMS_NOT_VALID_ERROR with no hint about the real cause. This is easy to hit when building the two JSON payloads programmatically (REST API instead of the UI): any code mismatch between the relations and the definitions on a pre task silently degrades into the generic error.

The existing size-mismatch check in generateTaskRelationList already returns TASK_DEFINE_NOT_EXIST for missing post task codes, but pre-task codes never reach it — the NPE happens first.

Brief change log

• WorkflowDefinitionServiceImpl#generateTaskRelationList: before calling transformTask, validate that every pre/post task code referenced by the relations has a matching task definition; throw TASK_DEFINE_NOT_EXIST (50030) naming the missing codes — the same semantics the existing post-code check already uses. This covers both the create and the update path.
• ProcessServiceImpl#transformTask: make the mapping null-safe (filter(Objects::nonNull) on the pre-task lookup, null-safe taskType) so that other callers of this service method degrade gracefully instead of throwing NPE.

Verify this pull request

This change added tests and can be verified as follows:

• Added testCreateWorkflowDefinitionShouldRejectUnknownPreTaskCode to WorkflowDefinitionServiceTest: a relation referencing an unknown preTaskCode now fails fast with TASK_DEFINE_NOT_EXIST and nothing is persisted.
• Ran mvn -pl dolphinscheduler-api -am test -Dtest=WorkflowDefinitionServiceTest locally — all tests in the class pass.

[boring-cyborg]

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

[SbloodyS]

Reject missing predecessors instead of silently removing them

ProcessServiceImpl.java:720 adds filter(Objects::nonNull), turning a relation such as missing A -> existing B into a node B with no predecessors.

The new validation only protects generateTaskRelationList(). WorkflowInstanceServiceImpl calls checkWorkflowNodeList() directly, which uses transformTask() without that validation. The filtered graph now passes validation, and saveTaskRelation() can persist the original dangling relation because it also does not reject missing definitions. Previously, this input failed during transformation.

Please validate missing task references in the shared transformation path and fail explicitly instead of dropping dependencies. Add regression coverage for workflow-instance updates, asserting that the missing code is reported and no invalid relations are persisted.

[wank125]

Thanks for the review @SbloodyS — good catch on the silent edge drop. Addressed in 00743a1:

• ProcessServiceImpl#transformTask now collects pre task codes without a matching task definition and throws an explicit exception naming the missing codes (shared path; the filter(Objects::nonNull) is gone)
• WorkflowDefinitionServiceImpl#checkWorkflowNodeList converts that failure into TASK_DEFINE_NOT_EXIST, so the workflow-instance update path reports the missing code instead of passing the filtered graph
• added regression tests: direct transformTask failure, checkWorkflowNodeList conversion, and updateWorkflowInstance asserting the missing code is reported and that neither saveTaskRelation nor saveWorkflowDefine persists anything when the check fails

The up-front validation in generateTaskRelationList is kept so the workflow-definition path still fails fast with the same status before reaching transformTask.

[SbloodyS]

Please remove the null taskType fallback to an empty string in ProcessServiceImpl#transformTask(). A missing task type is invalid data and should fail explicitly with task identity, rather than producing a TaskNode with an empty type. This fallback is unrelated to the missing-task-code fix.

[wank125]

Addressed in 5452fb0: the empty-string fallback for a null taskType is removed — transformTask now fails explicitly with the offending task's taskCode and taskName, consistent with the missing-pre-task-code handling. Regression suite (54 tests) passes.

[SbloodyS]

LGTM

[ruanwenjun]

It might be better to throw an invalid DAG status code here, and then specify in the exception message that the task definition is missing.

[ruanwenjun]

It's better to throw ServiceException at transformTask rather than wrap all exception into new ServiceException(Status.TASK_DEFINE_NOT_EXIST, e.getMessage());.

[wank125]

Thanks for the review @ruanwenjun — addressed in 8cc040c:

• transformTask keeps throwing its ServiceException with the precise reason (missing pre taskCodes, null taskType), and checkWorkflowNodeList now translates it to the new Status.WORKFLOW_DAG_IS_INVALID(50082) instead of wrapping everything into TASK_DEFINE_NOT_EXIST. The message states that task definitions are missing and names the codes. I kept a narrow catch (only the service-layer ServiceException) because it is not handled by ApiExceptionHandler, so letting it propagate would surface as INTERNAL_SERVER_ERROR — happy to drop the translation entirely if you prefer that trade-off.
• The relation/definition consistency checks in WorkflowDefinitionServiceImpl now use WORKFLOW_DAG_IS_INVALID as well, with the missing task codes in the message.
• The raw workflowTaskRelationJson is no longer logged in the error path, which also resolves the CodeQL log-injection alert.
• Regression suite (54 tests in WorkflowDefinitionServiceTest, 22 in WorkflowInstanceServiceTest) passes; spotless:check passes.