diff --git a/CHANGELOG.md b/CHANGELOG.md index a234701..19eb52b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 0.0.21 +* Changed `NoContent` to a typedef for `dynamic` to fix chopper integration with empty response bodies +* `JsonSerializableConverter` now throws a `JsonConverterException` instead of a `JsonUnsupportedObjectError`, so it can be caught by `tryCall` +* Exported `base_error.dart` so `BaseError` and its subtypes are reachable from the package barrel +* `JsonSerializableConverter` now logs deserialization failures via `BoltLogger`, tagged with the target type: `CheckedFromJsonException` at severe level (rethrown), `JsonConverterException` at warning level + ## 0.0.20 * Fixed bug in handschrift copyWith * Fixed minor bugs in BoltLogger diff --git a/example/lib/core/modules/chopper_module.dart b/example/lib/core/modules/chopper_module.dart index fde86c3..c8dfe65 100644 --- a/example/lib/core/modules/chopper_module.dart +++ b/example/lib/core/modules/chopper_module.dart @@ -1,6 +1,5 @@ import 'package:chopper/chopper.dart'; import 'package:dcc_toolkit/chopper/json_serializable_converter.dart'; -import 'package:dcc_toolkit/chopper/no_content.dart'; import 'package:example/profile/data/model/api_user_response.dart'; import 'package:flutter/foundation.dart'; import 'package:injectable/injectable.dart'; @@ -17,7 +16,4 @@ abstract class ChopperModule { ); } -const _factories = { - NoContent: NoContent.fromJson, - ApiUserResponse: ApiUserResponse.fromJson, -}; +const _factories = {ApiUserResponse: ApiUserResponse.fromJson}; diff --git a/example/pubspec.lock b/example/pubspec.lock index 06c1250..5fdc674 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -207,7 +207,7 @@ packages: path: ".." relative: true source: path - version: "0.0.19" + version: "0.0.21" equatable: dependency: transitive description: diff --git a/lib/chopper/chopper.dart b/lib/chopper/chopper.dart index f44dcb8..f408723 100644 --- a/lib/chopper/chopper.dart +++ b/lib/chopper/chopper.dart @@ -1,4 +1,6 @@ +export 'base_error.dart'; export 'color_converter.dart'; export 'datetime_converter.dart'; +export 'json_converter_exception.dart'; export 'json_serializable_converter.dart'; export 'no_content.dart'; diff --git a/lib/chopper/json_converter_exception.dart b/lib/chopper/json_converter_exception.dart new file mode 100644 index 0000000..f0c3d3c --- /dev/null +++ b/lib/chopper/json_converter_exception.dart @@ -0,0 +1,14 @@ +/// Thrown when a json response cannot be converted to the expected type. +class JsonConverterException implements Exception { + /// Creates a new [JsonConverterException]. + const JsonConverterException(this.type, {required this.message}); + + /// The type that could not be converted. + final Type type; + + /// A description of what went wrong. + final String message; + + @override + String toString() => 'JsonConverterException: $message'; +} diff --git a/lib/chopper/json_serializable_converter.dart b/lib/chopper/json_serializable_converter.dart index d3b8f6d..102a724 100644 --- a/lib/chopper/json_serializable_converter.dart +++ b/lib/chopper/json_serializable_converter.dart @@ -1,7 +1,10 @@ import 'dart:async'; -import 'dart:convert'; -import 'package:chopper/chopper.dart'; +import 'package:chopper/chopper.dart' hide Level; +import 'package:dcc_toolkit/chopper/json_converter_exception.dart'; +import 'package:dcc_toolkit/logger/bolt_logger.dart'; +import 'package:json_annotation/json_annotation.dart' show CheckedFromJsonException; +import 'package:logging/logging.dart'; /// Method signature for a function that creates a dart object from a json map. typedef JsonFactory = T Function(Map json); @@ -18,26 +21,39 @@ class JsonSerializableConverter extends JsonConverter { /// ```dart /// final jsonConverter = JsonSerializableConverter({ /// User: User.fromJson, - /// NoContent: NoContent.fromJson, /// }); /// ``` final Map> factories; + Never _logAndThrow(String message) { + final exception = JsonConverterException(T, message: message); + BoltLogger.zap(exception, tag: '$T', level: Level.WARNING); + + throw exception; + } + T _decodeMap(Map values) { final jsonFactory = factories[T]; if (jsonFactory == null) { - throw JsonUnsupportedObjectError(T, cause: 'No fromJson was registered for JsonSerializableConverter for $T'); + _logAndThrow('No fromJson was registered for JsonSerializableConverter for $T'); } if (jsonFactory is! JsonFactory) { - throw JsonUnsupportedObjectError(T, cause: 'fromJson type does not match $T'); + _logAndThrow('fromJson type does not match $T'); } - return jsonFactory(values); + try { + return jsonFactory(values); + } on CheckedFromJsonException catch (e, stackTrace) { + BoltLogger.shock([e, stackTrace], tag: '$T'); + rethrow; + } } List _decodeList(Iterable values) => values.nonNulls.map((v) => _decode(v) as T).toList(); dynamic _decode(dynamic entity) { + if (T == dynamic) return entity; + if (entity is Iterable) return _decodeList(entity as List); if (entity is Map) return _decodeMap(entity as Map); diff --git a/lib/chopper/no_content.dart b/lib/chopper/no_content.dart index 11b66ac..59292aa 100644 --- a/lib/chopper/no_content.dart +++ b/lib/chopper/no_content.dart @@ -1,10 +1,5 @@ -/// A class representing an empty response body. -class NoContent { - /// Creates a new [NoContent] instance. - const NoContent(); - - /// Creates a new [NoContent] instance from a json map. - // ignore unused parameter in factory constructor. This is done to match the method signature of the converter. - //ignore: avoid_unused_constructor_parameters - factory NoContent.fromJson(Map json) => const NoContent(); -} +/// A type representing an empty response body. +/// +/// Aliased to `dynamic` so chopper can handle empty (e.g. 204) response +/// bodies without requiring a registered `fromJson` factory. +typedef NoContent = dynamic; diff --git a/lib/common/result/result.dart b/lib/common/result/result.dart index e055ca8..be10538 100644 --- a/lib/common/result/result.dart +++ b/lib/common/result/result.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:chopper/chopper.dart'; import 'package:dcc_toolkit/chopper/base_error.dart'; +import 'package:dcc_toolkit/chopper/json_converter_exception.dart'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart'; import 'package:json_annotation/json_annotation.dart'; @@ -71,6 +72,7 @@ Future> tryCall(FutureOr Function() fn, {Future> Funct }), ClientException() => Result.failure(const NoInternetError()), CheckedFromJsonException() => Result.failure(const ServerError()), + JsonConverterException() => Result.failure(const ServerError()), _ => Result.failure(const UnknownError()), }; } diff --git a/pubspec.yaml b/pubspec.yaml index 7254a8e..259a73b 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: dcc_toolkit description: "Internal toolkit package used by the DCC team." -version: 0.0.20 +version: 0.0.21 homepage: https://dutchcodingcompany.com repository: https://github.com/DutchCodingCompany/dcc_toolkit diff --git a/test/chopper/json_serializable_converter_test.dart b/test/chopper/json_serializable_converter_test.dart index ee115ed..a59075e 100644 --- a/test/chopper/json_serializable_converter_test.dart +++ b/test/chopper/json_serializable_converter_test.dart @@ -1,27 +1,72 @@ -import 'dart:convert'; - -import 'package:chopper/chopper.dart'; +import 'package:chopper/chopper.dart' hide Level; +import 'package:dcc_toolkit/chopper/json_converter_exception.dart'; import 'package:dcc_toolkit/chopper/json_serializable_converter.dart'; +import 'package:dcc_toolkit/chopper/no_content.dart'; +import 'package:dcc_toolkit/logger/bolt_logger.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; +import 'package:json_annotation/json_annotation.dart' show CheckedFromJsonException; +import 'package:logging/logging.dart'; void main() { final baseResponse = http.Response('', 200); - test('Error is thrown when factory is missing', () { + late MemoryCharge memoryCharge; + + setUp(() { + memoryCharge = MemoryCharge(); + BoltLogger.charge([memoryCharge]); + }); + + tearDown(BoltLogger.discharge); + + test('Error is thrown when factory is missing', () async { final response = Response(baseResponse, '{"name":"John"}'); const converter = JsonSerializableConverter({}); - expect(() => converter.convertResponse(response), throwsA(isA())); + await expectLater( + converter.convertResponse(response), + throwsA(isA()), + ); + + expect(memoryCharge.items.length, 1); + expect(memoryCharge.items[0].origin.level, Level.WARNING); + expect(memoryCharge.items[0].origin.loggerName, 'TestModel'); + expect(memoryCharge.items[0].origin.error, isA()); }); - test('Error is thrown when wrong factory is added', () { + test('Error is thrown when wrong factory is added', () async { final response = Response(baseResponse, '{"name":"John"}'); const converter = JsonSerializableConverter({TestModel: TestModel2.fromJson}); - expect(() => converter.convertResponse(response), throwsA(isA())); + await expectLater( + converter.convertResponse(response), + throwsA(isA()), + ); + + expect(memoryCharge.items.length, 1); + expect(memoryCharge.items[0].origin.level, Level.WARNING); + expect(memoryCharge.items[0].origin.loggerName, 'TestModel'); + expect(memoryCharge.items[0].origin.error, isA()); + }); + + test('CheckedFromJsonException is logged and rethrown', () async { + final response = Response(baseResponse, '{"name":"John"}'); + + const converter = JsonSerializableConverter({ThrowingModel: ThrowingModel.fromJson}); + + await expectLater( + converter.convertResponse(response), + throwsA(isA()), + ); + + expect(memoryCharge.items.length, 1); + expect(memoryCharge.items[0].origin.level, Level.SEVERE); + expect(memoryCharge.items[0].origin.loggerName, 'ThrowingModel'); + expect(memoryCharge.items[0].origin.error, isA()); + expect(memoryCharge.items[0].origin.stackTrace, isNotNull); }); test('Uses model to convert json into model', () async { @@ -32,6 +77,7 @@ void main() { final result = await converter.convertResponse(response); expect(result.body?.name, 'John'); + expect(memoryCharge.items, isEmpty); }); test('Uses model to convert json into List', () async { @@ -44,6 +90,26 @@ void main() { expect(result.body?[0].name, 'John'); expect(result.body?[1].name, 'John2'); }); + + test('Handles an empty body for NoContent without a registered factory', () async { + final response = Response(baseResponse, ''); + + const converter = JsonSerializableConverter({}); + + final result = await converter.convertResponse(response); + + expect(result.body, ''); + }); + + test('Returns the raw body for NoContent when the response is not empty', () async { + final response = Response(baseResponse, '{"name":"John"}'); + + const converter = JsonSerializableConverter({}); + + final result = await converter.convertResponse(response); + + expect(result.body, {'name': 'John'}); + }); } class TestModel { @@ -65,3 +131,8 @@ class TestModel2 { Map toJson() => {'name': name}; } + +class ThrowingModel { + factory ThrowingModel.fromJson(Map json) => + throw CheckedFromJsonException(json, 'name', 'ThrowingModel', 'invalid'); +} diff --git a/test/common/result/try_call_test.dart b/test/common/result/try_call_test.dart index 539f9b2..0a91170 100644 --- a/test/common/result/try_call_test.dart +++ b/test/common/result/try_call_test.dart @@ -1,5 +1,6 @@ import 'package:chopper/chopper.dart' as c; import 'package:dcc_toolkit/chopper/base_error.dart'; +import 'package:dcc_toolkit/chopper/json_converter_exception.dart'; import 'package:dcc_toolkit/common/result/result.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; @@ -77,6 +78,7 @@ void main() { [c.ChopperHttpException(c.Response(http.Response('woepsie', 500), 'woepsie')), const ServerError()], [ClientException('woepsie'), const NoInternetError()], [CheckedFromJsonException({}, null, 'woepsie', null), const ServerError()], + [const JsonConverterException(int, message: 'woepsie'), const ServerError()], ], (Exception exception, BaseError expectedError) async { final result = await tryCall(() async => throw exception);