You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicecomb.apache.org by yhs0092 <yh...@163.com> on 2018/11/01 12:56:36 UTC

Re: [Discuss][JavaChassis] about customization of vertx org.apache.servicecomb.transport.rest.vertx.RestBodyHandler

Hi, I've tried to remove ServiceComb exceptions from RestBodyHandler today. Technically, it is feasible.
In my test, I replace the ServiceComb exceptions with IllegalArgumentException in the case that upload directory is not specified and replace with 413 status code in the case that uploaded file is too large. In GlobalRestFailureHandler I recognize the exception and status code and then return corresponding response.


This solution seems working well, but there are still some details need to be discussed. The first one is whether it's proper to use IllegalArgumentException in this situation. I use it because I cannot find other Exception more accurately express the meaning "upload directory is not specified". The other one is that the logic to convert the exceptions into error response is set in GlobalRestFailureHandler. It seems not elegant, but I still don't find any better solution.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 17:54,yhs0092<yh...@163.com> wrote:
Hi, one of the JavaChassis exception wrapped in RestBodyHandler is added by me.
I will try to remove the JavaChassis exceptions in RestBodyHandler and see whether it can works well.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 09:00,wjm wjm<zz...@gmail.com> wrote:
currently we
customized org.apache.servicecomb.transport.rest.vertx.RestBodyHandler
1.disable upload feature when uploadDir is null, this is a general
requirement
2.wrap some exceptions to JavaChassis exception, this is not a general
requirement

if there is only customization 1, maybe we can try to raise a PR to vertx
otherwise we always forget to merge latest vertx logic to our code.

maybe we can confirm if customization 2 is necessary.

Re: [Discuss][JavaChassis] about customization ofvertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler

Posted by yhs0092 <yh...@163.com>.
Yes, I've raised an issue[1] to ask about this feature.
Since we expect to remove org.apache.servicecomb.transport.rest.vertx.RestBodyHandler and use io.vertx.ext.web.handler.impl.BodyHandlerImpl directly, we need to ensure the behavior between these two handlers is consistent.
After determining which HTTP status code to be returned in the case that uploads dir is null, I will continue to work on this task.


[1] https://github.com/vert-x3/vertx-web/issues/1051


Yours sincerely


Yao Haishi
yhs0092@163.com


On 11/6/2018 10:28,bismy<bi...@qq.com> wrote:
Yes you are right. My understanding is that "the request method XXX is not allowed in upload", so that it can be used.
We are thinking about PR to vert.x, maybe we can create a PR for them to review.




------------------ 原始邮件 ------------------
发件人: "yhs0092"<yh...@163.com>;
发送时间: 2018年11月5日(星期一) 晚上10:21
收件人: "dev@servicecomb.apache.org"<de...@servicecomb.apache.org>;

主题: Re:   [Discuss][JavaChassis] about customization ofvertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler



Hi @bismy@qq.com , I've searched for the descriptions about HTTP status 405, but the description[1] seems to indicate that this status is for those cases that request method is wrong, e.g. the request method is GET but the server only accept POST. So is it proper to use 405 in this scenario?
If there is no proper HTTP status, maybe we can use 500?


And I've create a task[2] to track this issue.


[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405
[2] https://issues.apache.org/jira/browse/SCB-1010


Yours sincerely


Yao Haishi
yhs0092@163.com


On 11/5/2018 20:24,bismy<bi...@qq.com> wrote:
Maybe we can send a


405 Method Not Allowed


message.


------------------ 原始邮件 ------------------
发件人: "yhs0092"<yh...@163.com>;
发送时间: 2018年11月1日(星期四) 晚上8:56
收件人: "dev@servicecomb.apache.org"<de...@servicecomb.apache.org>;

主题: Re:  [Discuss][JavaChassis] about customization of vertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler



Hi, I've tried to remove ServiceComb exceptions from RestBodyHandler today. Technically, it is feasible.
In my test, I replace the ServiceComb exceptions with IllegalArgumentException in the case that upload directory is not specified and replace with 413 status code in the case that uploaded file is too large. In GlobalRestFailureHandler I recognize the exception and status code and then return corresponding response.


This solution seems working well, but there are still some details need to be discussed. The first one is whether it's proper to use IllegalArgumentException in this situation. I use it because I cannot find other Exception more accurately express the meaning "upload directory is not specified". The other one is that the logic to convert the exceptions into error response is set in GlobalRestFailureHandler. It seems not elegant, but I still don't find any better solution.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 17:54,yhs0092<yh...@163.com> wrote:
Hi, one of the JavaChassis exception wrapped in RestBodyHandler is added by me.
I will try to remove the JavaChassis exceptions in RestBodyHandler and see whether it can works well.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 09:00,wjm wjm<zz...@gmail.com> wrote:
currently we
customized org.apache.servicecomb.transport.rest.vertx.RestBodyHandler
1.disable upload feature when uploadDir is null, this is a general
requirement
2.wrap some exceptions to JavaChassis exception, this is not a general
requirement

if there is only customization 1, maybe we can try to raise a PR to vertx
otherwise we always forget to merge latest vertx logic to our code.

maybe we can confirm if customization 2 is necessary.

回复: [Discuss][JavaChassis] about customization ofvertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler

Posted by bismy <bi...@qq.com>.
Yes you are right. My understanding is that "the request method XXX is not allowed in upload", so that it can be used. 
We are thinking about PR to vert.x, maybe we can create a PR for them to review. 




------------------ 原始邮件 ------------------
发件人: "yhs0092"<yh...@163.com>;
发送时间: 2018年11月5日(星期一) 晚上10:21
收件人: "dev@servicecomb.apache.org"<de...@servicecomb.apache.org>;

主题: Re:   [Discuss][JavaChassis] about customization ofvertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler



Hi @bismy@qq.com , I've searched for the descriptions about HTTP status 405, but the description[1] seems to indicate that this status is for those cases that request method is wrong, e.g. the request method is GET but the server only accept POST. So is it proper to use 405 in this scenario?
If there is no proper HTTP status, maybe we can use 500?


And I've create a task[2] to track this issue.


[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405
[2] https://issues.apache.org/jira/browse/SCB-1010


Yours sincerely


Yao Haishi
yhs0092@163.com


On 11/5/2018 20:24,bismy<bi...@qq.com> wrote:
Maybe we can send a


405 Method Not Allowed


message.


------------------ 原始邮件 ------------------
发件人: "yhs0092"<yh...@163.com>;
发送时间: 2018年11月1日(星期四) 晚上8:56
收件人: "dev@servicecomb.apache.org"<de...@servicecomb.apache.org>;

主题: Re:  [Discuss][JavaChassis] about customization of vertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler



Hi, I've tried to remove ServiceComb exceptions from RestBodyHandler today. Technically, it is feasible.
In my test, I replace the ServiceComb exceptions with IllegalArgumentException in the case that upload directory is not specified and replace with 413 status code in the case that uploaded file is too large. In GlobalRestFailureHandler I recognize the exception and status code and then return corresponding response.


This solution seems working well, but there are still some details need to be discussed. The first one is whether it's proper to use IllegalArgumentException in this situation. I use it because I cannot find other Exception more accurately express the meaning "upload directory is not specified". The other one is that the logic to convert the exceptions into error response is set in GlobalRestFailureHandler. It seems not elegant, but I still don't find any better solution.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 17:54,yhs0092<yh...@163.com> wrote:
Hi, one of the JavaChassis exception wrapped in RestBodyHandler is added by me.
I will try to remove the JavaChassis exceptions in RestBodyHandler and see whether it can works well.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 09:00,wjm wjm<zz...@gmail.com> wrote:
currently we
customized org.apache.servicecomb.transport.rest.vertx.RestBodyHandler
1.disable upload feature when uploadDir is null, this is a general
requirement
2.wrap some exceptions to JavaChassis exception, this is not a general
requirement

if there is only customization 1, maybe we can try to raise a PR to vertx
otherwise we always forget to merge latest vertx logic to our code.

maybe we can confirm if customization 2 is necessary.

Re: [Discuss][JavaChassis] about customization of vertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler

Posted by yhs0092 <yh...@163.com>.
Hi @bismy@qq.com , I've searched for the descriptions about HTTP status 405, but the description[1] seems to indicate that this status is for those cases that request method is wrong, e.g. the request method is GET but the server only accept POST. So is it proper to use 405 in this scenario?
If there is no proper HTTP status, maybe we can use 500?


And I've create a task[2] to track this issue.


[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405
[2] https://issues.apache.org/jira/browse/SCB-1010


Yours sincerely


Yao Haishi
yhs0092@163.com


On 11/5/2018 20:24,bismy<bi...@qq.com> wrote:
Maybe we can send a


405 Method Not Allowed


message.


------------------ 原始邮件 ------------------
发件人: "yhs0092"<yh...@163.com>;
发送时间: 2018年11月1日(星期四) 晚上8:56
收件人: "dev@servicecomb.apache.org"<de...@servicecomb.apache.org>;

主题: Re:  [Discuss][JavaChassis] about customization of vertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler



Hi, I've tried to remove ServiceComb exceptions from RestBodyHandler today. Technically, it is feasible.
In my test, I replace the ServiceComb exceptions with IllegalArgumentException in the case that upload directory is not specified and replace with 413 status code in the case that uploaded file is too large. In GlobalRestFailureHandler I recognize the exception and status code and then return corresponding response.


This solution seems working well, but there are still some details need to be discussed. The first one is whether it's proper to use IllegalArgumentException in this situation. I use it because I cannot find other Exception more accurately express the meaning "upload directory is not specified". The other one is that the logic to convert the exceptions into error response is set in GlobalRestFailureHandler. It seems not elegant, but I still don't find any better solution.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 17:54,yhs0092<yh...@163.com> wrote:
Hi, one of the JavaChassis exception wrapped in RestBodyHandler is added by me.
I will try to remove the JavaChassis exceptions in RestBodyHandler and see whether it can works well.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 09:00,wjm wjm<zz...@gmail.com> wrote:
currently we
customized org.apache.servicecomb.transport.rest.vertx.RestBodyHandler
1.disable upload feature when uploadDir is null, this is a general
requirement
2.wrap some exceptions to JavaChassis exception, this is not a general
requirement

if there is only customization 1, maybe we can try to raise a PR to vertx
otherwise we always forget to merge latest vertx logic to our code.

maybe we can confirm if customization 2 is necessary.

回复: [Discuss][JavaChassis] about customization of vertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler

Posted by bismy <bi...@qq.com>.
Maybe we can send a
   

405 Method Not Allowed


message. 


------------------ 原始邮件 ------------------
发件人: "yhs0092"<yh...@163.com>;
发送时间: 2018年11月1日(星期四) 晚上8:56
收件人: "dev@servicecomb.apache.org"<de...@servicecomb.apache.org>;

主题: Re:  [Discuss][JavaChassis] about customization of vertxorg.apache.servicecomb.transport.rest.vertx.RestBodyHandler



Hi, I've tried to remove ServiceComb exceptions from RestBodyHandler today. Technically, it is feasible.
In my test, I replace the ServiceComb exceptions with IllegalArgumentException in the case that upload directory is not specified and replace with 413 status code in the case that uploaded file is too large. In GlobalRestFailureHandler I recognize the exception and status code and then return corresponding response.


This solution seems working well, but there are still some details need to be discussed. The first one is whether it's proper to use IllegalArgumentException in this situation. I use it because I cannot find other Exception more accurately express the meaning "upload directory is not specified". The other one is that the logic to convert the exceptions into error response is set in GlobalRestFailureHandler. It seems not elegant, but I still don't find any better solution.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 17:54,yhs0092<yh...@163.com> wrote:
Hi, one of the JavaChassis exception wrapped in RestBodyHandler is added by me.
I will try to remove the JavaChassis exceptions in RestBodyHandler and see whether it can works well.


Yours sincerely


Yao Haishi
yhs0092@163.com


On 10/31/2018 09:00,wjm wjm<zz...@gmail.com> wrote:
currently we
customized org.apache.servicecomb.transport.rest.vertx.RestBodyHandler
1.disable upload feature when uploadDir is null, this is a general
requirement
2.wrap some exceptions to JavaChassis exception, this is not a general
requirement

if there is only customization 1, maybe we can try to raise a PR to vertx
otherwise we always forget to merge latest vertx logic to our code.

maybe we can confirm if customization 2 is necessary.