Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make ReturnOverrides more flexible when returning errors #2693

Closed
matiasinsaurralde opened this issue Nov 25, 2019 · 4 comments
Closed

Make ReturnOverrides more flexible when returning errors #2693

matiasinsaurralde opened this issue Nov 25, 2019 · 4 comments
Labels
enhancement

Comments

@matiasinsaurralde
Copy link
Contributor

Is your feature request related to a problem? Please describe.

  1. Currently when using rich plugins, it's possible to override the response by using ReturnOverrides, this structure takes two parameters: HTTP status code and body.

  2. When the HTTP status code is higher than 400 and a body like "test" is used, the generated response body looks like this: {"error": "test"}.

  3. It would be more flexible if we can allow the user to fully customize this behavior in error scenarios. We could keep the current functionality by default (and for compatibility purposes) and introduce a flag like disable_json_error or a similar name.
    When this flag is set to true, with the same parameters specified in 2) we would get response body like "test".

  4. This could be useful for cases where it's required to fully customize the error output.
    When using status codes lower than 400 we don't really add any JSON wrapper, so the user already has full flexibility in this case.

Describe the solution you'd like
The easiest solution would be to add a flag to the ReturnOverrides data structure. This will help us with compatibility too.
@sedkis
Copy link
Contributor

sedkis commented Nov 26, 2019 •
edited
Loading

What about also letting us choose the return Code?
like in here: #2652

@buger
Copy link
Member

buger commented Nov 26, 2019

It is already possible and part of ReturnOverrides structure

matiasinsaurralde added a commit to matiasinsaurralde/tyk that referenced this issue Dec 11, 2019
@matiasinsaurralde
coprocess: make ReturnOverrides more flexible when returning errors (T…
6cf6d3b 
…ykTechnologies#2693)
matiasinsaurralde added a commit to matiasinsaurralde/tyk that referenced this issue Dec 11, 2019
@matiasinsaurralde
coprocess: make ReturnOverrides more flexible when returning errors (T…
79dac19 
…ykTechnologies#2693)
@matiasinsaurralde matiasinsaurralde mentioned this issue Dec 11, 2019
Merged
matiasinsaurralde added a commit to matiasinsaurralde/tyk that referenced this issue Dec 11, 2019
@matiasinsaurralde
coprocess: make ReturnOverrides more flexible when returning errors (T…
9c48235 
…ykTechnologies#2693)
buger pushed a commit that referenced this issue Dec 23, 2019
@matiasinsaurralde @buger
coprocess: make ReturnOverrides more flexible when returning errors (#…
42f2393 
…2693) (#2731)

For #2693, WIP so don't merge yet!
@furkansenharputlu
Copy link
Contributor

Fixed by #2731

matiasinsaurralde added a commit to matiasinsaurralde/tyk that referenced this issue Jan 7, 2020
@matiasinsaurralde
coprocess: make ReturnOverrides more flexible when returning errors (T…
4fdaf57 
…ykTechnologies#2693)
matiasinsaurralde added a commit to matiasinsaurralde/tyk that referenced this issue Jan 7, 2020
@matiasinsaurralde
coprocess: make ReturnOverrides more flexible when returning errors (T…
283e549 
…ykTechnologies#2693)
@buger buger mentioned this issue Jan 10, 2020
Closed
@buger buger reopened this Jan 20, 2020
@buger buger mentioned this issue Jan 20, 2020
Merged
buger added a commit that referenced this issue Jan 21, 2020
@buger
Fix return overrides to override error bodies (#2815)
3fd2820 
The previous PR #2731 contained an error, and always forced return custom error: which brakes backward compatibility.
In this PR I added tests covering all return overrides functionality and normalized its behavior for JSVM and Python.

Additionally, it use reverse logic now, and plugin needs set `ReturnOverrides.OverrideError` (JS) or `return_overrides.override_error` (Python) to `true` in order to override the body of response.

Additionally, I renamed `response_error` field to `response_body`, because it makes way more sense.
The old field still can be set for backward compatibility.

#2693
@buger
Copy link
Member

buger commented Jan 21, 2020

I added one more fix to this, details inside this PR #2815

buger added a commit that referenced this issue Jan 21, 2020
@buger
Fix return overrides to override error bodies (#2815)
ea166d2 
The previous PR #2731 contained an error, and always forced return custom error: which brakes backward compatibility.
In this PR I added tests covering all return overrides functionality and normalized its behavior for JSVM and Python.

Additionally, it use reverse logic now, and plugin needs set `ReturnOverrides.OverrideError` (JS) or `return_overrides.override_error` (Python) to `true` in order to override the body of response.

Additionally, I renamed `response_error` field to `response_body`, because it makes way more sense.
The old field still can be set for backward compatibility.

#2693
@buger buger closed this as completed Jan 22, 2020
@l3ender l3ender mentioned this issue Mar 5, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@buger @matiasinsaurralde @sedkis @furkansenharputlu