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.

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

Allow override of maxBodyBytesLength in ErrorDecoder #2113

Merged
merged 4 commits into from
Jul 5, 2023

Conversation

ddeath
Copy link
Contributor

@ddeath ddeath commented Jul 3, 2023

Allow override of maxBodyBytesLength, maxBodyCharsLength in ErrorDecoder

Currently if we use Default error decoder from this lib it will cut error messages so we cant really see them in the logs. This will allow us to configure max error message length.

Added ability to specify our own max values in Default error decoder.

Let me know what you think about this

@vitalijr2
Copy link
Collaborator

vitalijr2 commented Jul 3, 2023

IMHO:

1. ErrorDecoder is called by ResponseHandler that is used by *MethodHandler. Users usually do not call it manually. We do not call it in any Client's child.
2. There is the (default) decode method that have two parameters that it ignores. For me it is bad way.
3. Why do we need to pass maxBodyBytesLength and maxBodyCharsLength each time when we call the decode method? I guess these values are the same.

If the question "How to set maxBodyBytesLength and maxBodyCharsLength" the solution is "use the constructor": we can add another constructor that gives these parameters and passes them to errorStatus(String methodKey, Response response, Integer maxBodyBytesLength, Integer maxBodyCharsLength).

As far as I see you have implemented it :)

@ddeath
Copy link
Contributor Author

ddeath commented Jul 3, 2023

IMHO:

1. ErrorDecoder is called by ResponseHandler that is used by *MethodHandler. Users usually do not call it manually. We do not call it in any Client's child. 2. There is the (default) decode method that have two parameters that it ignores. For me it is bad way. 3. Why do we need to pass maxBodyBytesLength and maxBodyCharsLength each time when we call the decode method? I guess these values are the same.

If the question "How to set maxBodyBytesLength and maxBodyCharsLength" the solution is "use the constructor": we can add another constructor that gives these parameters and passes them to errorStatus(String methodKey, Response response, Integer maxBodyBytesLength, Integer maxBodyCharsLength).

As far as I see you have implemented it :)

Yeah I realized later that via constructor it is much simpler and does not require breaking change

@velo velo merged commit 7d4eb18 into OpenFeign:master Jul 5, 2023
@ddeath ddeath deleted the feature/allow-max-body-override branch July 5, 2023 08:54
velo pushed a commit that referenced this pull request Oct 7, 2024
* Allow override of maxBodyBytesLength in ErrorDecoder

* Apply linter

* Choose constructor approach

* Reformat
velo pushed a commit that referenced this pull request Oct 8, 2024
* Allow override of maxBodyBytesLength in ErrorDecoder

* Apply linter

* Choose constructor approach

* Reformat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants