Skip to content

Conversation

@ColinZou
Copy link

@ColinZou ColinZou commented Dec 12, 2024

…return null and decide(workflowId) will not be invoked

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR
Added null check for return value taskResult.getLogs before saving task execution log when update task.

…return null and decide(workflowId) will not be invoked
@ColinZou
Copy link
Author

@jmigueprieto could you please check this PR. The input logs of TaskResult can be null somehow and nullpointer exception throws.

@jmigueprieto
Copy link
Contributor

@jmigueprieto could you please check this PR. The input logs of TaskResult can be null somehow and nullpointer exception throws.

Hey @ColinZou - Thanks for bringing this up. I'll take a look.

Copy link
Contributor

@jmigueprieto jmigueprieto left a comment

Choose a reason for hiding this comment

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

  • Add a test or explain the execution path that leads to a NPE
  • Change Optional usage.

@ColinZou
Copy link
Author

I found this NPE when invoking update task from rest API. The update task API accepts logs=null and the NPE get triggered in the place I fixed.

@v1r3n
Copy link
Collaborator

v1r3n commented Apr 4, 2025

  • Add a test or explain the execution path that leads to a NPE
  • Change Optional usage.

Hi @ColinZou can you take care of the comments and we can merge.

@ColinZou
Copy link
Author

ColinZou commented Apr 4, 2025

  • Add a test or explain the execution path that leads to a NPE
    I've reply the message. this one:
    I found this NPE when invoking update task from rest API. The update task API accepts logs=null and the NPE get triggered in the place I fixed.
  • Change Optional usage.
    It was DONE at Feb 5 2025.

Hi @ColinZou can you take care of the comments and we can merge.

@v1r3n I am really not sure which comment left to be resolved. Let me know if I misunderstood anything either from you or @jmigueprieto please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Ready for release

Development

Successfully merging this pull request may close these issues.

4 participants