[Fix][Zeta] Fix NPE in JobInfoService

[dybyte]

Fixes: #10695

Purpose of this pull request

This PR fixes a potential NPE in JobInfoService.

Does this PR introduce any user-facing change?

How was this patch tested?

Added JobInfoServiceNullSafetyTest.

Check list

• If any new Jar binary package adding in your PR, please add License Notice according
  New License Guide
• If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
• If necessary, please update incompatible-changes.md to describe the incompatibility caused by this PR.
• If you are contributing the connector code, please check that the following files are updated:
  1. Update plugin-mapping.properties and add new connector information in it
  2. Update the pom file of seatunnel-dist
  3. Add ci label in label-scope-conf
  4. Add e2e testcase in seatunnel-e2e
  5. Update connector plugin_config

[DanielLeens]

I pulled the branch locally and traced the finished-job path in JobInfoService.getJobInfoJson(Long).

I found one blocking gap: this patch fixes the finishedJobMetrics == null case, but the same path can still NPE when finishedJobDAGInfo is missing.

On the new code:

• JobInfoService.getJobInfoJson() still loads finishedJobDAGInfo from IMAP_FINISHED_JOB_VERTEX_INFO and passes it straight through (JobInfoService.java:86-94)
• BaseService.getJobInfoJson(...) then unconditionally calls jobDAGInfo.toJsonObject() (BaseService.java:242-268)

So if the finished-job state exists but the DAG info map entry is absent, we still crash on the same REST path. The new test class only covers missing metrics (JobInfoServiceNullSafetyTest.java:89-101), not missing DAG info.

Please either make the finished-job JSON builder null-safe for jobDAGInfo as well, or prove by invariant that IMAP_FINISHED_JOB_STATE and IMAP_FINISHED_JOB_VERTEX_INFO are always populated atomically. Right now the NPE is only narrowed, not eliminated.

[dybyte]

I pulled the branch locally and traced the finished-job path in JobInfoService.getJobInfoJson(Long).

I found one blocking gap: this patch fixes the finishedJobMetrics == null case, but the same path can still NPE when finishedJobDAGInfo is missing.

On the new code:

• JobInfoService.getJobInfoJson() still loads finishedJobDAGInfo from IMAP_FINISHED_JOB_VERTEX_INFO and passes it straight through (JobInfoService.java:86-94)
• BaseService.getJobInfoJson(...) then unconditionally calls jobDAGInfo.toJsonObject() (BaseService.java:242-268)

So if the finished-job state exists but the DAG info map entry is absent, we still crash on the same REST path. The new test class only covers missing metrics (JobInfoServiceNullSafetyTest.java:89-101), not missing DAG info.

Please either make the finished-job JSON builder null-safe for jobDAGInfo as well, or prove by invariant that IMAP_FINISHED_JOB_STATE and IMAP_FINISHED_JOB_VERTEX_INFO are always populated atomically. Right now the NPE is only narrowed, not eliminated.

@DanielLeens I checked this again. JobDAGInfo.toJsonObject() itself is not null-safe, but in this path BaseService.getJobInfoJson(...) already checks whether jobDAGInfo is null before calling toJsonObject().

So I don't think a missing finishedJobDAGInfo would trigger an NPE here. The null-safety issue in this patch is limited to finishedJobMetrics on this path.

[DanielLeens]

Hi @dybyte, thank you for checking my previous comment so carefully. I re-pulled the latest head locally and traced the REST path again. You are right: my earlier concern about finishedJobDAGInfo was not valid for the current code path, because BaseService.getJobInfoJson(...) already has a null-safe fallback before calling jobDAGInfo.toJsonObject().

What This PR Fixes

• User pain point: when the REST job-info endpoint reads a finished job whose state exists but whose metrics entry is missing, the old code dereferenced finishedJobMetrics directly and could fail with an NPE instead of returning basic job information.
• Fix approach: JobInfoService.getJobInfoJson(Long) now falls back to JobMetrics.empty() when the finished-job metrics map has no entry for the job.
• One-line summary: finished-job REST lookup now survives missing metrics and still returns a valid JSON response.

Runtime Path Checked

REST job-info request
  -> RestHttpGetCommandProcessor / JobInfoServlet
      -> JobInfoService.getJobInfoJson(jobId)
          -> read IMAP_RUNNING_JOB_INFO
              -> if present: convert running JobInfo to JSON
              -> if absent: continue to finished-job state
          -> read IMAP_FINISHED_JOB_STATE
              -> if absent: return a JsonObject with only jobId
              -> if present: read metrics and DAG info
          -> read IMAP_FINISHED_JOB_METRICS
              -> old behavior: null metrics -> finishedJobMetrics.toJsonString() NPE
              -> new behavior: null metrics -> JobMetrics.empty()
          -> read IMAP_FINISHED_JOB_VERTEX_INFO
          -> BaseService.getJobInfoJson(jobState, metricsJson, dagInfo)
              -> jobDAGInfo != null ? jobDAGInfo.toJsonObject() : new JsonObject()
              -> getJobMetrics(...) also checks jobDAGInfo before reading vertex info

Key Findings

• The normal finished-job REST path does hit this PR.
• The real fixed case is: finished state exists, but finished metrics are missing.
• This is a precise fix and matches the existing JobMetrics.empty() semantics already used elsewhere in the engine history path.
• The new test class covers the missing-metrics regression, the normal finished state + metrics + DAG path, and the no-finished-state fallback.
• Current GitHub checks are green.

Compatibility / Side Effects

Compatibility: fully compatible.

No API, option, default value, protocol, serialization, checkpoint, or historical upgrade behavior is changed. The only user-visible effect is that a REST query that previously failed on this edge case can now return basic finished-job information with empty metrics.

Findings

No blocking issue found in the current head.

Merge Decision

Conclusion: can merge

1. Blocking items

• None.

2. Recommended non-blocking improvements

• None.

Overall, the current implementation is focused and low risk. Thanks again for the clear follow-up; after retracing the actual call chain, I agree this PR is ready from my side.

[davidzollo]

+1
Good job

[DanielLeens]

Hi @dybyte, I rechecked the current head locally again after the latest review activity.

My conclusion is unchanged:

• the real fixed case is still “finished state exists, but finished metrics are missing”
• the current head falls back to JobMetrics.empty()
• the regression test covers the important null-safety branches
• the current Build is green

Runtime chain I rechecked

REST job-info request
  -> JobInfoService.getJobInfoJson(jobId)
      -> running job map
      -> finished job state map
      -> finished metrics map
          -> null -> JobMetrics.empty()
      -> convert to JSON

Conclusion: can merge

Blocking items:

• None from my side.

Thanks again for the careful follow-up here. The current implementation still looks focused and low risk.