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

Improve exception message for DioException.badResponse #1998

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Improve exception message for DioException.badResponse #1998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dio/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ See the [Migration Guide][] for the complete breaking changes list.**
## Unreleased

- Raise warning for `Map`s other than `Map<String, dynamic>` when encoding request data.
- Improve exception messages

## 5.3.3

Expand Down
37 changes: 35 additions & 2 deletions dio/lib/src/dio_exception.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ class DioException implements Exception {
}) =>
DioException(
type: DioExceptionType.badResponse,
message: 'The request returned an '
'invalid status code of $statusCode.',
message: _badResponseExceptionMessage(statusCode),
requestOptions: requestOptions,
response: response,
error: null,
Expand Down Expand Up @@ -211,4 +210,38 @@ class DioException implements Exception {
}
return msg;
}

/// Because of [ValidateStatus] we need to consider all status codes when
/// creating a [DioException.badResponse].
static String _badResponseExceptionMessage(int statusCode) {
var message = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to write final String message if it's declared in every single flow.

if (statusCode >= 100 && statusCode < 200) {
message =
'This is an informational response - the request was received, continuing processing';
} else if (statusCode >= 200 && statusCode < 300) {
message =
'The request was successfully received, understood, and accepted';
} else if (statusCode >= 300 && statusCode < 400) {
message =
'Redirection: further action needs to be taken in order to complete the request';
} else if (statusCode >= 400 && statusCode < 500) {
message =
'Client error - the request contains bad syntax or cannot be fulfilled';
} else if (statusCode >= 500 && statusCode < 600) {
message =
'Server error - the server failed to fulfil an apparently valid request';
} 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.';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be a duplicated dot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I rather want the message to be helpful than concise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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/Status

Here it should be "server's software.".\nRead more, right? It's "server's software."\n.Read more currently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which made that dot redundant

}

return ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe StringBuffer would be a good choice at here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really aware of the benefits of a StringBuffer. Can you explain why it would improve this code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

'This exception was thrown because the response has a status code of $statusCode '
'and RequestOptions.validateStatus was configured to throw for this status code.\n'
'The status code of $statusCode has the following meaning: "$message"\n.'
'Read more about status codes at https://developer.mozilla.org/en-US/docs/Web/HTTP/Status\n'
'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.';
}
}