[Flink] Fix BuildPipeline.execStep NPE on steps that return null - #4486
Merged
wolfboys merged 1 commit intoAug 13, 2026
Merged
Conversation
… in BuildPipeline execStep() wrapped every successful step result in Optional.of(result), which throws NullPointerException whenever a step's Callable legitimately returns null -- and returning null is the normal case for steps that only perform a side effect (e.g. "create/clean the build workspace") and have nothing meaningful to hand to the next step. This is a textbook Scala-to-Java migration bug: the original Scala code almost certainly used Option(x), which tolerates null, not the strict Optional.of(x). Every one of BuildPipeline's ~20 call sites already followed the same execStep(seq, ...).orElseThrow(() -> pipelineException()) pattern purely to turn a failed step into a thrown exception -- Optional was being used as a success/failure signal, not as a genuine "maybe absent" value, so it could never actually represent "succeeded, but there's no value" once any step returned null. FlinkRemoteBuildPipeline's step 1 hit exactly that: it succeeded (the workspace really was created), but execStep threw NullPointerException at Optional.of(null) before the SUCCESS status update even had a chance to matter, and the console reported the pipeline as failed building the app jar for every Flink SQL application. Change execStep() to return R directly and throw pipelineException() itself on failure -- the exact exception every caller was already constructing via orElseThrow(). This removes the null ambiguity entirely: a normal return means success (whatever the value, including null), and failure is always signaled by an exception, matching how the callers actually use it. Drop the now redundant .orElseThrow(...) at all 20 call sites across BuildPipeline#runYarnSqlBuildSteps, FlinkRemoteBuildPipeline, FlinkK8sSessionBuildPipeline, AbstractK8sApplicationBuildPipeline, FlinkK8sApplicationBuildPipeline, and SparkK8sApplicationBuildPipeline. Reproduced via POST /flink/pipe/build against a real Flink 2.2.1 standalone cluster: the build pipeline for a Flink SQL application always failed at step 1/2 with this exact NullPointerException, confirmed via the stdout log (streampark.out; this exception is thrown before the console's own GlobalExceptionHandler ever sees a request, so it never reaches the ERROR log file or the persisted ApplicationBuildPipeline.errorSummary).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request
Issue Number: close #4481
Fixes
BuildPipeline#execStep()throwingNullPointerExceptionwhenever a pipeline step'sCallablelegitimately returnsnull— which is the normal case for steps that only perform a side effect (e.g. "create/clean the build workspace") and have nothing meaningful to hand to the next step. This breaks the build/release pipeline for a Flink SQL application (and, per the ~20 affected call sites, potentially every deploy mode) at its very first step.Brief change log
BuildPipeline#execStep(): returnRdirectly instead ofOptional<R>, and throwpipelineException()directly from thecatchblock on failure, instead of returningOptional.empty()for the caller to detect via.orElseThrow(...). This removes the null ambiguity entirely — JavaOptionalcannot distinguish "empty because the step failed" from "empty because it succeeded with anullvalue," which is exactly what made a one-lineOptional.of→Optional.ofNullableswap insufficient (it would just make.orElseThrow()mis-fire on every successful null-returning step instead of crashing atOptional.of)..orElseThrow(() -> pipelineException())/.orElseThrow(this::pipelineException)at all ~20 call sites:BuildPipeline#runYarnSqlBuildSteps,FlinkRemoteBuildPipeline,FlinkK8sSessionBuildPipeline,AbstractK8sApplicationBuildPipeline,FlinkK8sApplicationBuildPipeline,SparkK8sApplicationBuildPipeline.No caller had extra logic in its
orElseThrowlambda beyondthrow pipelineException();— every occurrence checked and removed identically.Verifying this change
Manually verified against a real Flink 2.2.1 standalone cluster:
POST /flink/pipe/buildfor a Flink SQL application always fails at step 1/2 ("Create building workspace") withNullPointerExceptionatOptional.of(null), even though the workspace directory is genuinely created on disk. The failure is invisible via the REST API (hasError: false,errorSummary: null) and only shows up in raw stdout (streampark.out), not the console's own error log — because the exception is thrown from a background thread pool task beforeGlobalExceptionHandleris ever involved.POST /flink/pipe/buildfor the same application completes withpipeStatus: SUCCESSfor both steps, and produces a real 6.2 MB shaded jar atworkspace/workspace/<appId>/streampark-flinkjob_<jobName>.jar../mvnw -pl streampark-flink/streampark-flink-packer,streampark-console/streampark-console-service -am clean compile checkstyle:check spotless:checkpasses with 0 violations across all touched modules.Does this pull request potentially affect one of the following parts
BuildPipelinecontract used only withinstreampark-flink-packerand its console caller;execStep/runYarnSqlBuildStepsareprotected, not part of any public/external API)