You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/10/14 08:06:18 UTC

[GitHub] [apisix] tokers opened a new issue, #8091: feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned

tokers opened a new issue, #8091:
URL: https://github.com/apache/apisix/issues/8091

   ### Description
   
   Currently, the `limit-*` plugins support configuring an `error_msg` field to let users customize the error response when requests are throttled. However, Apache APISIX always puts the `error_msg` into a JSON string. It's not reasonable if users want to configure an HTML page.
   
   https://github.com/apache/apisix/blob/4e25e5b4acc2e8293787b8861d0b4c2715cdc3af/apisix/plugins/limit-count/init.lua#L289-L293


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned [apisix]

Posted by "smileby (via GitHub)" <gi...@apache.org>.
smileby commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1886182926

   > Will the `error_response` in https://github.com/apache/apisix/blob/master/docs/en/latest/terminology/plugin.md#plugin-common-configuration solve the problem?
   
   I think this is not user friendly,This approach is more like a helpless move. 🙃


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned [apisix]

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1886343968

   > > Will the `error_response` in https://github.com/apache/apisix/blob/master/docs/en/latest/terminology/plugin.md#plugin-common-configuration solve the problem?
   > 
   > I think this is not user friendly,This approach is more like a helpless move. 🙃
   
   @smileby 
   Why do you think this approach is "helpless"?


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on issue #8091: feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1278894414

   Will the `error_response` in https://github.com/apache/apisix/blob/master/docs/en/latest/terminology/plugin.md#plugin-common-configuration solve the problem?


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned [apisix]

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1886951254

   > > > > Will the `error_response` in https://github.com/apache/apisix/blob/master/docs/en/latest/terminology/plugin.md#plugin-common-configuration solve the problem?
   > > > 
   > > > 
   > > > I think this is not user friendly,This approach is more like a helpless move. 🙃
   > > 
   > > 
   > > @smileby Why do you think this approach is "helpless"?
   > 
   > Fields in the plugin configuration cannot be used to configure return messages. You need to use the general functions of the plug-in. May be difficult for users to understand simply because of a hard-coded problem
   
   I could not understand this quite well. Could you provide an example for it? Do you mean generating the response on the fly instead of using a configured string?
   
   As for hardcode, the configuration with error response:
   ```
   {
       "x": {
           "_meta": {
               "error_response": {
                   "message": "Missing credential in request"
               }
           },
          "reject_msg": "Missing credential in request"
       }
   }
   ```
   
   is not quite different from the configuration field `reject_msg`. They are both hardcoded strings.
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] tokers commented on issue #8091: feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1279940602

   > Will the `error_response` in https://github.com/apache/apisix/blob/master/docs/en/latest/terminology/plugin.md#plugin-common-configuration solve the problem?
   
   Yes. It do solve this problem.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned [apisix]

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1891355327

   > Hi, @spacewander 😃 thank you for your reply.
   > 
   > What I want to express is the following:
   > 
   > For the limit-* plugins, the return structure is currently defined
   > 
   > ```
   >      return conf.rejected_code, { error_msg = conf.rejected_msg }
   > ```
   > 
   > Assume that the user plug-in configuration is as follows:
   > 
   > ```
   >      conf.rejected_code = 200,
   >      conf.rejected_msg = "Please try again later"
   > ```
   > 
   > When the corresponding rule is triggered, the request will return:
   > 
   > ```
   >      http code : 200
   >      reponse msg : {"error_msg": "Please try again later"}
   > ```
   > 
   > In most scenarios, the front-end logic requires the API to return a standard structure for processing, and they will expect to return customized data, such as:
   > 
   > ```
   >      http code : 200
   >      reponse msg : {"code": "0010", "desc": "Please try again later"}
   > ```
   > 
   > So, limit-* plugins should just return the error_msg without any wrapping. This is more convenient for most users. Even if _meta does what I'm saying.
   > 
   > If a user wants to return a JSON string, just let this user configure this JSON string. So the same for HTML or others.
   > 
   > In the documentation of each plug-in, there is almost no dedicated description of the _meta attribute. Users may need to consult more information to deal with this issue.
   > 
   > So I think it would be most convenient to adjust the return structure of the plug-in.
   > 
   > I want to hear your thoughts 😊 ~~~
   
   I see. Thanks for your explanation! The thing I worry about is that the change in returning messages will be a break change. That is part of the reason that we introduced `error_response`.
   
   Let's wait for the current maintainer to decide if the break change is worthwhile.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] tokers commented on issue #8091: feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1279940721

   I think for a while, and I think we need to change the description of the `error_msg` field. Tell users that it will be a field in the JSON string.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned [apisix]

Posted by "smileby (via GitHub)" <gi...@apache.org>.
smileby commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1886873445

   > > > Will the `error_response` in https://github.com/apache/apisix/blob/master/docs/en/latest/terminology/plugin.md#plugin-common-configuration solve the problem?
   > > 
   > > 
   > > I think this is not user friendly,This approach is more like a helpless move. 🙃
   > 
   > @smileby Why do you think this approach is "helpless"?
   
   Fields in the plugin configuration cannot be used to configure return messages. You need to use the general functions of the plug-in. 
   May be difficult for users to understand simply because of a hard-coded problem


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] starsz commented on issue #8091: feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned

Posted by GitBox <gi...@apache.org>.
starsz commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1278781580

   Good.
   So we should two field, like "reject_body" and "reject_content_type" ?


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] feat: As a user, I want the `limit-*` plugins can return the `error_msg` directly, so that HTML or other kinds of response can be returned [apisix]

Posted by "smileby (via GitHub)" <gi...@apache.org>.
smileby commented on issue #8091:
URL: https://github.com/apache/apisix/issues/8091#issuecomment-1890336062

   Hi,  @spacewander  :smiley: thank you for your reply.
   
   What I want to express is the following:
   
   For the limit-* plugins, the return structure is currently defined
   ```
        return conf.rejected_code, { error_msg = conf.rejected_msg }
   ```
   
   Assume that the user plug-in configuration is as follows:
   
   ```
        conf.rejected_code = 200,
        conf.rejected_msg = "Please try again later"
   ```
   
   
   When the corresponding rule is triggered, the request will return:
   
   ```
        http code : 200
        reponse msg : {"error_msg": "Please try again later"}
   ```
   
   In most scenarios, the front-end logic requires the API to return a standard structure for processing, and they will expect to return customized data, such as:
   
   ```
        http code : 200
        reponse msg : {"code": "0010", "desc": "Please try again later"}
   ```
   
   So, limit-* plugins should just return the error_msg without any wrapping. This is more convenient for most users. Even if _meta does what I'm saying.
   
   If a user wants to return a JSON string, just let this user configure this JSON string. So the same for HTML or others.
   
   In the documentation of each plug-in, there is almost no dedicated description of the _meta attribute. Users may need to consult more information to deal with this issue. 
   
   So I think it would be most convenient to adjust the return structure of the plug-in. 
   
   
   I want to hear your thoughts  :blush:   ~~~


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org