From cccf6964827b484246123b7fc62d97f1eb9b1c89 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 19 Dec 2024 15:59:20 -0800 Subject: [PATCH 1/3] api [nfc]: Reorder NetworkException to before sibling classes We're about to group the others together under a new sibling of this class, so it'll be cleanest if this one doesn't appear in the midst of the others. --- lib/api/exception.dart | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/api/exception.dart b/lib/api/exception.dart index cda75b4b07..ef9951ef47 100644 --- a/lib/api/exception.dart +++ b/lib/api/exception.dart @@ -28,6 +28,23 @@ sealed class ApiRequestException implements Exception { String toString() => message; } +/// A network-level error that prevented even getting an HTTP response. +class NetworkException extends ApiRequestException { + /// The exception describing the underlying error. + /// + /// This can be any exception value that [http.Client.send] throws. + /// Ideally that would always be an [http.ClientException], + /// but empirically it can be [TlsException] and possibly others. + final Object cause; + + NetworkException({required super.routeName, required super.message, required this.cause}); + + @override + String toString() { + return 'NetworkException: $message ($cause)'; + } +} + /// An error returned through the Zulip server API. /// /// See API docs: https://zulip.com/api/rest-error-handling @@ -69,23 +86,6 @@ class ZulipApiException extends ApiRequestException { } } -/// A network-level error that prevented even getting an HTTP response. -class NetworkException extends ApiRequestException { - /// The exception describing the underlying error. - /// - /// This can be any exception value that [http.Client.send] throws. - /// Ideally that would always be an [http.ClientException], - /// but empirically it can be [TlsException] and possibly others. - final Object cause; - - NetworkException({required super.routeName, required super.message, required this.cause}); - - @override - String toString() { - return 'NetworkException: $message ($cause)'; - } -} - /// Some kind of server-side error in handling the request. /// /// This should always represent either some kind of operational issue From 4b937bbddeaa3bdfb3ad44ee85739a270451e4a8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 19 Dec 2024 16:13:33 -0800 Subject: [PATCH 2/3] api [nfc]: Add HttpException as base-class home for httpStatus field --- lib/api/exception.dart | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/api/exception.dart b/lib/api/exception.dart index ef9951ef47..d4bebeeb62 100644 --- a/lib/api/exception.dart +++ b/lib/api/exception.dart @@ -28,7 +28,10 @@ sealed class ApiRequestException implements Exception { String toString() => message; } -/// A network-level error that prevented even getting an HTTP response. +/// A network-level error that prevented even getting an HTTP response +/// to some Zulip API network request. +/// +/// This is the antonym of [HttpException]. class NetworkException extends ApiRequestException { /// The exception describing the underlying error. /// @@ -45,19 +48,26 @@ class NetworkException extends ApiRequestException { } } -/// An error returned through the Zulip server API. +/// Some kind of [ApiRequestException] that came as an HTTP response. /// -/// See API docs: https://zulip.com/api/rest-error-handling -class ZulipApiException extends ApiRequestException { - - /// The Zulip API error code returned by the server. - final String code; - +/// This is the antonym of [NetworkException]. +sealed class HttpException extends ApiRequestException { /// The HTTP status code returned by the server. /// - /// This is always in the range 400..499. + /// On [ZulipApiException], this is always in the range 400..499. final int httpStatus; + HttpException({required super.routeName, required this.httpStatus, required super.message}); +} + +/// An error returned through the Zulip server API, +/// and with a 4xx HTTP status code. +/// +/// See API docs: https://zulip.com/api/rest-error-handling +class ZulipApiException extends HttpException { + /// The Zulip API error code returned by the server. + final String code; + /// The error's JSON data, if any, beyond the properties common to all errors. /// /// This consists of the properties other than `result`, `code`, and `msg`. @@ -67,8 +77,8 @@ class ZulipApiException extends ApiRequestException { ZulipApiException({ required super.routeName, + required super.httpStatus, required this.code, - required this.httpStatus, required this.data, required super.message, }) : assert(400 <= httpStatus && httpStatus <= 499); @@ -91,9 +101,7 @@ class ZulipApiException extends ApiRequestException { /// This should always represent either some kind of operational issue /// on the server, or a bug in the server where its responses don't /// agree with the documented API. -sealed class ServerException extends ApiRequestException { - final int httpStatus; - +sealed class ServerException extends HttpException { /// The response body, decoded as a JSON object. /// /// This is null if the body could not be read, or was not a valid JSON object. @@ -101,7 +109,7 @@ sealed class ServerException extends ApiRequestException { ServerException({ required super.routeName, - required this.httpStatus, + required super.httpStatus, required this.data, required super.message, }); From 531af691d1866a1ff1728c941d15ddfd61d20903 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 19 Dec 2024 16:17:16 -0800 Subject: [PATCH 3/3] store [nfc]: Use HttpException to simplify error cases This expresses more directly what we really mean here: if the response had either the HTTP status code or the Zulip API error code that mean a rate-limit error, treat it as a rate-limit error. --- lib/model/store.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/model/store.dart b/lib/model/store.dart index 58a8a70615..72773b39ce 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -1128,8 +1128,7 @@ class UpdateMachine { case Server5xxException(): shouldReportToUser = true; - case ServerException(httpStatus: 429): - case ZulipApiException(httpStatus: 429): + case HttpException(httpStatus: 429): case ZulipApiException(code: 'RATE_LIMIT_HIT'): // TODO(#946) handle rate-limit errors more generally, in ApiConnection shouldReportToUser = true;