You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/09/09 22:52:58 UTC
[GitHub] [incubator-ratis] amaliujia opened a new pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
amaliujia opened a new pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194
## What changes were proposed in this pull request?
Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient.
Detailed analysis see https://issues.apache.org/jira/browse/RATIS-1058.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1058
## How was this patch tested?
N/A
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia closed pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia closed pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691522697
@runzhiwang
Can you take a look at this PR please?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691861144
> @amaliujia Thanks the patch. But how response == null happens ?
This is more like a pre-defence check. The NULL can happen when a new value added to the ENUM but the code in the SWITCH is not updated. I think it is always good to eliminate potential NPE.
Alternately, we can choose to throw an exception for a new type, WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691856582
@amaliujia Thanks the patch. But how response == null happens ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia closed pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia closed pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691863701
@amaliujia I think we fix NPE only when it could happen sometimes, otherwise we need check too much in the code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691865463
That's ok. This is a very minor change anyway.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691522697
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia closed pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia closed pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia closed pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia closed pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia edited a comment on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia edited a comment on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-691861144
> @amaliujia Thanks the patch. But how response == null happens ?
This is more like a pre-defence check. The NULL can happen when a new value added to the ENUM but the code in the SWITCH is not updated. I think it is always good to eliminate potential NPE.
Alternately, we can choose to throw an exception for a new type, WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #194: RATIS-1058. Avoid NPE when response is null in CombinedClientProtocolServerSideTranslatorPB.sendClient
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #194:
URL: https://github.com/apache/incubator-ratis/pull/194#issuecomment-689865344
@runzhiwang can you take a look? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org