Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Fixed NullpointerException in couple of places #983

Merged
merged 3 commits into from
Feb 12, 2019
Merged

Fixed NullpointerException in couple of places #983

merged 3 commits into from
Feb 12, 2019

Conversation

falu2010-netflix
Copy link
Contributor

@falu2010-netflix falu2010-netflix commented Feb 8, 2019 •
edited
Loading

  • Added precondition check to determine exact place for NPE
  • Fixed NPE in WebApplicationExceptionMapper

@codecov
Copy link

codecov bot commented Feb 8, 2019 •
edited
Loading

Codecov Report

Merging #983 into dev will decrease coverage by 0.02%.
The diff coverage is 75%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #983      +/-   ##
============================================
- Coverage     62.25%   62.22%   -0.03%     
+ Complexity     2557     2556       -1     
============================================
  Files           225      225              
  Lines         12888    12899      +11     
  Branches       1281     1284       +3     
============================================
+ Hits           8024     8027       +3     
- Misses         4154     4161       +7     
- Partials        710      711       +1
Impacted Files Coverage Δ Complexity Δ
...lix/conductor/core/execution/WorkflowExecutor.java 71.2% <0%> (-0.37%) 115 <0> (ø)
...ductor/server/resources/WebAppExceptionMapper.java 100% <100%> (ø) 3 <0> (ø) ⬇️
...conductor/core/metadata/MetadataMapperService.java 95.71% <100%> (+0.55%) 24 <13> (ø) ⬇️
.../java/com/netflix/conductor/service/Lifecycle.java 30% <0%> (-30%) 2% <0%> (-1%)
.../com/netflix/conductor/grpc/server/GRPCServer.java 78.57% <0%> (-14.29%) 4% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 787f856...e3eb561. Read the comment docs.

@coveralls
Copy link

coveralls commented Feb 8, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 2357

  • 10 of 12 (83.33%) changed or added relevant lines in 3 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.03%) to 68.244%
Changes Missing Coverage Covered Lines Changed/Added Lines %
core/src/main/java/com/netflix/conductor/core/execution/WorkflowExecutor.java 1 3 33.33%
Files with Coverage Reduction New Missed Lines %
grpc-server/src/main/java/com/netflix/conductor/grpc/server/GRPCServer.java 2 85.71%
core/src/main/java/com/netflix/conductor/service/Lifecycle.java 3 30.0%
Totals Coverage Status
Change from base Build 2351: -0.03%
Covered Lines: 9011
Relevant Lines: 13204
💛 - Coveralls

@falu2010-netflix falu2010-netflix requested review from cyzhao, kishorebanala, a user, s3nthil, pctreddy and cooniur and removed request for cyzhao February 8, 2019 02:34
kishorebanala
kishorebanala reviewed Feb 8, 2019
View reviewed changes
core/src/main/java/com/netflix/conductor/core/execution/WorkflowExecutor.java
@@ -978,6 +978,10 @@ public void addTaskToQueue(Task task) {
public void executeSystemTask(WorkflowSystemTask systemTask, String taskId, int unackTimeout) {
try {
Task task = executionDAOFacade.getTaskById(taskId);
if (task == null){
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering why executionDAO would return null instead of throwing error in the first place.

@falu2010-netflix falu2010-netflix merged commit 947f0d6 into dev Feb 12, 2019
@apanicker-nflx apanicker-nflx deleted the fix/exception branch February 20, 2019 21:27
long-64 pushed a commit to long-64/conductor that referenced this pull request Oct 2, 2019
@falu2010-netflix
Fixed NullpointerException in couple of places (Netflix#983)
7ab010c 
* Fixed NullpointerException

* Fixed NullpointerException

* Fixed NullpointerException
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kishorebanala kishorebanala kishorebanala left review comments

@apanicker-nflx apanicker-nflx apanicker-nflx approved these changes

@s3nthil s3nthil Awaiting requested review from s3nthil

@cyzhao cyzhao Awaiting requested review from cyzhao

@pctreddy pctreddy Awaiting requested review from pctreddy

@cooniur cooniur Awaiting requested review from cooniur

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@falu2010-netflix @coveralls @kishorebanala @apanicker-nflx