Skip to content

Commit

Permalink
[dio] Fix missing source stacktrace in DioError (#1722)
Browse files Browse the repository at this point in the history
[Improve nullability in
DioMixin.assureResponse](8418c72)
* there should never be a case where there are not `RequestOptions`
available
* this is technically a breaking change but we should instead mark this
`@internal` in the future

[Fix missing source stacktrace in
DioError](4f4ffe4)

* a `DioError` should always have a meaningful stacktrace which points
to the actual invocation of `Dio.get/post/xxx`
* `Dio` historically lost a lot of this source information due to a
multitude of asynchronous calls
* now the source stacktrace is consistently being set into the
`RequestOptions` instance and can later be retrieved and used when a
`DioError` is constructed
* additionally a `DioError` may contain another cause which may itself
contain a separate stackTrace with more detailed information


### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [x] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

### Additional context and info (if any)

The existing behavior which was somewhat correct got lost in
#1405 which was merged in the temporary
diox repository.

---------

Signed-off-by: Alex Li <[email protected]>
Co-authored-by: Alex Li <[email protected]>
  • Loading branch information
kuhnroyal and AlexV525 committed Mar 27, 2023
1 parent d4442a2 commit a75693a
Show file tree
Hide file tree
Showing 15 changed files with 1,120 additions and 92 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ plugins/http2_adapter/test/*_pinning.txt

.vscode/

# FVM
.fvm

# Miscellaneous
.DS_Store

Expand Down
6 changes: 6 additions & 0 deletions .idea/dio.iml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions dio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
- Fix double-completion when using `connectionTimeout` on web platform.
- Allow defining adapter methods through their constructors.
- Fix `FormData` encoding regression for maps with dynamic keys, introduced in 5.0.3.
- Mark several static `DioMixin` functions as `@internal`.
- Make `DioError.stackTrace` non-nullable.
- Ensure `DioError.stackTrace` always points to the correct call site.

## 5.0.3

Expand Down
3 changes: 1 addition & 2 deletions dio/lib/src/adapters/io_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class IOHttpClientAdapter implements HttpClientAdapter {
options.headers.forEach((k, v) {
if (v != null) request.headers.set(k, v);
});
} on SocketException catch (e, stackTrace) {
} on SocketException catch (e) {
if (!e.message.contains('timed out')) {
rethrow;
}
Expand All @@ -84,7 +84,6 @@ class IOHttpClientAdapter implements HttpClientAdapter {
httpClient.connectionTimeout ??
Duration.zero,
error: e,
stackTrace: stackTrace,
);
}

Expand Down
7 changes: 6 additions & 1 deletion dio/lib/src/cancel_token.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,13 @@ class CancelToken {

/// Cancel the request with the given [reason].
void cancel([Object? reason]) {
if (requestOptions == null) {
throw StateError(
'CancelToken was canceled before being used in a request.',
);
}
_cancelError = DioError.requestCancelled(
requestOptions: requestOptions ?? RequestOptions(),
requestOptions: requestOptions!,
reason: reason,
stackTrace: StackTrace.current,
);
Expand Down
14 changes: 7 additions & 7 deletions dio/lib/src/dio/dio_for_native.dart
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ class DioForNative with DioMixin implements Dio {
if (cancelToken == null || !cancelToken.isCancelled) {
subscription.resume();
}
}).catchError((dynamic e, StackTrace s) async {
}).catchError((Object e) async {
try {
await subscription.cancel();
} finally {
completer.completeError(
DioMixin.assureDioError(e, response.requestOptions, s),
DioMixin.assureDioError(e, response.requestOptions),
);
}
});
Expand All @@ -151,18 +151,18 @@ class DioForNative with DioMixin implements Dio {
closed = true;
await raf.close();
completer.complete(response);
} catch (e, s) {
} catch (e) {
completer.completeError(
DioMixin.assureDioError(e, response.requestOptions, s),
DioMixin.assureDioError(e, response.requestOptions),
);
}
},
onError: (e, s) async {
onError: (e) async {
try {
await closeAndDelete();
} finally {
completer.completeError(
DioMixin.assureDioError(e, response.requestOptions, s),
DioMixin.assureDioError(e, response.requestOptions),
);
}
},
Expand All @@ -183,7 +183,7 @@ class DioForNative with DioMixin implements Dio {
throw DioError.receiveTimeout(
timeout: timeout,
requestOptions: response.requestOptions,
stackTrace: s,
error: e,
);
} else {
throw e;
Expand Down
123 changes: 72 additions & 51 deletions dio/lib/src/dio_error.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,78 +56,99 @@ extension _DioErrorTypeExtension on DioErrorType {
class DioError implements Exception {
/// Prefer using one of the other constructors.
/// They're most likely better fitting.
const DioError({
DioError({
required this.requestOptions,
this.response,
this.type = DioErrorType.unknown,
this.error,
StackTrace? stackTrace,
this.message,
}) : stackTrace = identical(stackTrace, StackTrace.empty) ? null : stackTrace;
}) : stackTrace = identical(stackTrace, StackTrace.empty)
? requestOptions.sourceStackTrace ?? StackTrace.current
: stackTrace ??
requestOptions.sourceStackTrace ??
StackTrace.current;

const DioError.badResponse({
factory DioError.badResponse({
required int statusCode,
required this.requestOptions,
required this.response,
}) : type = DioErrorType.badResponse,
message = 'The request returned an '
required RequestOptions requestOptions,
required Response response,
}) =>
DioError(
type: DioErrorType.badResponse,
message: 'The request returned an '
'invalid status code of $statusCode.',
error = null,
stackTrace = null;
requestOptions: requestOptions,
response: response,
error: null,
);

const DioError.connectionTimeout({
factory DioError.connectionTimeout({
required Duration timeout,
required this.requestOptions,
this.error,
StackTrace? stackTrace,
}) : type = DioErrorType.connectionTimeout,
message = 'The request connection took '
required RequestOptions requestOptions,
Object? error,
}) =>
DioError(
type: DioErrorType.connectionTimeout,
message: 'The request connection took '
'longer than $timeout. It was aborted.',
response = null,
stackTrace =
identical(stackTrace, StackTrace.empty) ? null : stackTrace;
requestOptions: requestOptions,
response: null,
error: error,
);

const DioError.sendTimeout({
factory DioError.sendTimeout({
required Duration timeout,
required this.requestOptions,
}) : type = DioErrorType.sendTimeout,
message = 'The request took '
required RequestOptions requestOptions,
}) =>
DioError(
type: DioErrorType.sendTimeout,
message: 'The request took '
'longer than $timeout to send data. It was aborted.',
response = null,
error = null,
stackTrace = null;
requestOptions: requestOptions,
response: null,
error: null,
);

const DioError.receiveTimeout({
factory DioError.receiveTimeout({
required Duration timeout,
required this.requestOptions,
StackTrace? stackTrace,
}) : type = DioErrorType.receiveTimeout,
message = 'The request took '
required RequestOptions requestOptions,
Object? error,
}) =>
DioError(
type: DioErrorType.receiveTimeout,
message: 'The request took '
'longer than $timeout to receive data. It was aborted.',
response = null,
error = null,
stackTrace =
identical(stackTrace, StackTrace.empty) ? null : stackTrace;
requestOptions: requestOptions,
response: null,
error: error,
);

const DioError.requestCancelled({
required this.requestOptions,
factory DioError.requestCancelled({
required RequestOptions requestOptions,
required Object? reason,
StackTrace? stackTrace,
}) : type = DioErrorType.cancel,
message = 'The request was cancelled.',
error = reason,
response = null,
stackTrace =
identical(stackTrace, StackTrace.empty) ? null : stackTrace;

const DioError.connectionError({
required this.requestOptions,
}) =>
DioError(
type: DioErrorType.cancel,
message: 'The request was cancelled.',
requestOptions: requestOptions,
response: null,
error: reason,
stackTrace: stackTrace,
);

factory DioError.connectionError({
required RequestOptions requestOptions,
required String reason,
}) : type = DioErrorType.connectionError,
message = 'The connection errored: $reason',
response = null,
error = null,
stackTrace = null;
}) =>
DioError(
type: DioErrorType.connectionError,
message: 'The connection errored: $reason',
requestOptions: requestOptions,
response: null,
error: null,
);

/// The request info for the request that throws exception.
final RequestOptions requestOptions;
Expand All @@ -144,7 +165,7 @@ class DioError implements Exception {

/// The stacktrace of the original error/exception object;
/// It's usually not null when `type` is [DioErrorType.unknown].
final StackTrace? stackTrace;
final StackTrace stackTrace;

/// The error message that throws a [DioError].
final String? message;
Expand Down
Loading

0 comments on commit a75693a

Please sign in to comment.