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

Keep DioException.toString clean !!! #2287

Open
narumi147 opened this issue Aug 19, 2024 · 5 comments · May be fixed by #2296 or #2297
Open

Keep DioException.toString clean !!! #2287

narumi147 opened this issue Aug 19, 2024 · 5 comments · May be fixed by #2296 or #2297
Labels
s: feature This issue indicates a feature request

Comments

@narumi147
Copy link
Contributor

narumi147 commented Aug 19, 2024

Request Statement

To be honest, the new added detail message about the error reason, is useless for developer. Such as:

This exception was thrown because the response has a status code of 504 and RequestOptions.validateStatus was configured to throw for this status code.
The status code of 504 has the following meaning: "Server error - the server failed to fulfil an apparently valid request"
Read more about status codes at https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
In order to resolve this exception you typically have either to verify and fix your request code or you have to fix the server code.

In most cases, developer will log error to console or file, these messages are useless and annoying. Which developer doesn't know the http code meaning? If one kind of error keeps raising, the log will be filled with these useless texts.

Solution Brainstorm

Ensure toString only print "necessary" info, and print extra helpful info in another method like toDetailString.

So if want to show user the error detail, we could just call DioException.toDetailString. If we want to print or logging, normal toString is enough.

@narumi147 narumi147 added the s: feature This issue indicates a feature request label Aug 19, 2024
@narumi147
Copy link
Contributor Author

To ignore these useless text, I have to dig into each logging process, each error print process to "Remove" these message, or even ignore entire DioException.

@AlexV525
Copy link
Member

@ueman Do you want to evaluate the request? I do think that is fair enough, and if we need to change this, we might also consider to support #1949.

@ueman
Copy link
Contributor

ueman commented Aug 22, 2024

It was introduced since we had a lot of people in the past creating issues for exceptions being thrown due to bad HTTP status codes.

So in contrast to what the issue says, a lot of people do indeed not know HTTP status codes.

I probably wouldn't introduce something like the suggestion but rather make it more verbose in debug mode vs release mode.

That being said, I would like to further understand what the issue with being verbose is.

In most cases, developer will log error to console or file, these messages are useless and annoying. Which developer doesn't know the http code meaning?

I realise that this is a rethorical question, but people who just start out don't know, and for those these kinds of messages are super helpful. Not everyone knows as much as you or I.

@narumi147
Copy link
Contributor Author

@ueman maybe we can add a configuration, such as static field Dio.loggingMode, then decide the logging format according to this field value. by deafult, it can be set to detail logging

@AlexV525
Copy link
Member

I've filed 2 PRs to find out if there is a better answer for the issue. Please vote or add comments if you'd like to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: feature This issue indicates a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants