Skip to content

Commit f240caf

Browse files
gnpricechrisbobbe
authored andcommitted
api: On parse failure, include details in exception
If the server's response is malformed, as with the server issue that led to #383, the existing logging is unhelpful for working out what the problem is, even in debug: we learn what request failed, but not where in the response the misparse was. When that issue happened, I debugged it with a cruder version of this change, just augmenting this `catch` block to print the exception and its stack trace. But we can do better, using the magic of Error.throwWithStackTrace.
1 parent 2d92d80 commit f240caf

File tree

5 files changed

+54
-6
lines changed

5 files changed

+54
-6
lines changed

assets/l10n/app_en.arb

+8
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,14 @@
319319
"httpStatus": {"type": "int", "example": "200"}
320320
}
321321
},
322+
"errorMalformedResponseWithCause": "Server gave malformed response; HTTP status {httpStatus}; {details}",
323+
"@errorMalformedResponseWithCause": {
324+
"description": "Error message when an API call fails because we could not parse the response, with details of the failure.",
325+
"placeholders": {
326+
"httpStatus": {"type": "int", "example": "200"},
327+
"details": {"type": "String", "example": "type 'Null' is not a subtype of type 'String' in type cast"}
328+
}
329+
},
322330
"errorRequestFailed": "Network request failed: HTTP status {httpStatus}",
323331
"@errorRequestFailed": {
324332
"description": "Error message when an API call fails.",

lib/api/core.dart

+6-3
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,12 @@ class ApiConnection {
112112

113113
try {
114114
return fromJson(json);
115-
} catch (e) {
116-
throw MalformedServerResponseException(
117-
routeName: routeName, httpStatus: httpStatus, data: json);
115+
} catch (exception, stackTrace) { // TODO(log)
116+
Error.throwWithStackTrace(
117+
MalformedServerResponseException(
118+
routeName: routeName, httpStatus: httpStatus, data: json,
119+
causeException: exception),
120+
stackTrace);
118121
}
119122
}
120123

lib/api/exception.dart

+14-2
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,22 @@ class Server5xxException extends ServerException {
112112
///
113113
/// See docs: https://zulip.com/api/rest-error-handling
114114
class MalformedServerResponseException extends ServerException {
115+
/// The underlying exception from trying to parse the response, when applicable.
116+
///
117+
/// This should be paired with the use of [Error.throwWithStackTrace]
118+
/// in order to preserve the underlying exception's stack trace, which
119+
/// may be more informative than the exception object itself.
120+
final Object? causeException;
121+
115122
MalformedServerResponseException({
116123
required super.routeName,
117124
required super.httpStatus,
118125
required super.data,
119-
}) : super(message: GlobalLocalizations.zulipLocalizations
120-
.errorMalformedResponse(httpStatus));
126+
this.causeException,
127+
}) : super(message: causeException == null
128+
? GlobalLocalizations.zulipLocalizations
129+
.errorMalformedResponse(httpStatus)
130+
: GlobalLocalizations.zulipLocalizations
131+
.errorMalformedResponseWithCause(
132+
httpStatus, causeException.toString()));
121133
}

test/api/core_test.dart

+25
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,31 @@ void main() {
279279
await checkMalformed( json: {}, fromJson: (json) => json['x'] as String);
280280
await checkMalformed( json: {'x': 3}, fromJson: (json) => json['x'] as String);
281281
});
282+
283+
test('malformed API success responses: exception preserves details', () async {
284+
int distinctivelyNamedFromJson(Map<String, dynamic> json) {
285+
throw DistinctiveError("something is wrong");
286+
}
287+
288+
try {
289+
await tryRequest(json: {}, fromJson: distinctivelyNamedFromJson);
290+
assert(false);
291+
} catch (e, st) {
292+
check(e).isA<MalformedServerResponseException>()
293+
..causeException.isA<DistinctiveError>()
294+
..message.contains("something is wrong");
295+
check(st.toString()).contains("distinctivelyNamedFromJson");
296+
}
297+
});
298+
}
299+
300+
class DistinctiveError extends Error {
301+
final String message;
302+
303+
DistinctiveError(this.message);
304+
305+
@override
306+
String toString() => message;
282307
}
283308

284309
Future<T> tryRequest<T>({

test/api/exception_checks.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,5 @@ extension Server5xxExceptionChecks on Subject<Server5xxException> {
2626
}
2727

2828
extension MalformedServerResponseExceptionChecks on Subject<MalformedServerResponseException> {
29-
// no properties not on ServerException
29+
Subject<Object?> get causeException => has((e) => e.causeException, 'causeException');
3030
}

0 commit comments

Comments
 (0)