Improve exception message for DioException.badResponse #1998
Conversation
|
Nice, just think we should still print the actual status code somewhere. |
It does right in the first line of the exception message: |
|
My bad... |
| } else { | ||
| message = | ||
| 'A response with a status code that is not within the range of inclusive 100 to exclusive 600' | ||
| 'is a non-standard response, possibly due to the server\'s software.'; |
There was a problem hiding this comment.
Yeah, but I rather want the message to be helpful than concise.
There was a problem hiding this comment.
Yeah, but I rather want the message to be helpful than concise.
Hmm, what I mean is the last dot of possibly due to the server\'s software. might be redundant.
There was a problem hiding this comment.
Ah I see, yeah, that might be redundant, but this message is aimed at people who are not yet that knowledgeable of the HTTP spec and so on. So I think it's a rather helpful addition, even if it might seem superfluous for us knowledgeable people.
There was a problem hiding this comment.
DioException [bad response]: This exception was thrown because the response has a status code of 0 and RequestOptions.validateStatus was configured to throw for this status code.
The status code of 0 has the following meaning: "A response with a status code that is not within the range of inclusive 100 to exclusive 600is a non-standard response, possibly due to the server's software."
.Read more about status codes at https://developer.mozilla.org/en-US/docs/Web/HTTP/StatusHere it should be "server's software.".\nRead more, right? It's "server's software."\n.Read more currently.
| /// Because of [ValidateStatus] we need to consider all status codes when | ||
| /// creating a [DioException.badResponse]. | ||
| static String _badResponseExceptionMessage(int statusCode) { | ||
| var message = ''; |
There was a problem hiding this comment.
I used to write final String message if it's declared in every single flow.
| 'is a non-standard response, possibly due to the server\'s software.'; | ||
| } | ||
|
|
||
| return '' |
There was a problem hiding this comment.
Maybe StringBuffer would be a good choice at here.
There was a problem hiding this comment.
I'm not really aware of the benefits of a StringBuffer. Can you explain why it would improve this code?
There was a problem hiding this comment.
I'm not really aware of the benefits of a
StringBuffer. Can you explain why it would improve this code?
You just don't have to constantly write \n then.
|
@AlexV525 I plan to do another round of exception message improvements for the other exceptions. I'll incorporate your feedback in that PR, okay? |
Yeah sure. I'm available now |
Since we receive a lot of issues around the topic of status code, I think we should improve message of it. So I've made it more descriptinve and actionable.
New Pull Request Checklist
mainbranch to avoid conflicts (via merge from master or rebase)CHANGELOG.mdin the corresponding packageAdditional context and info (if any)