🐛 Change NoContent to typedef for dynamic

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

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};

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:

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';

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';
+}

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> = T Function(Map<String, dynamic> json);
@@ -18,26 +21,39 @@ class JsonSerializableConverter extends JsonConverter {
   /// ```dart
   /// final jsonConverter = JsonSerializableConverter({
   ///  User: User.fromJson,
-  ///  NoContent: NoContent.fromJson,
   ///  });
   ///  ```
   final Map<Type, JsonFactory<dynamic>> factories;
 
+  Never _logAndThrow<T>(String message) {
+    final exception = JsonConverterException(T, message: message);
+    BoltLogger.zap(exception, tag: '$T', level: Level.WARNING);
+
+    throw exception;
+  }
+
   T _decodeMap<T>(Map<String, dynamic> values) {
     final jsonFactory = factories[T];
     if (jsonFactory == null) {
-      throw JsonUnsupportedObjectError(T, cause: 'No fromJson was registered for JsonSerializableConverter for $T');
+      _logAndThrow<T>('No fromJson was registered for JsonSerializableConverter for $T');
     }
     if (jsonFactory is! JsonFactory<T>) {
-      throw JsonUnsupportedObjectError(T, cause: 'fromJson type does not match $T');
+      _logAndThrow<T>('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<T> _decodeList<T>(Iterable<dynamic> values) => values.nonNulls.map<T>((v) => _decode<T>(v) as T).toList();
 
   dynamic _decode<T>(dynamic entity) {
+    if (T == dynamic) return entity;
+
     if (entity is Iterable) return _decodeList<T>(entity as List);
 
     if (entity is Map) return _decodeMap<T>(entity as Map<String, dynamic>);

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<String, dynamic> 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;

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<Result<S>> tryCall<S>(FutureOr<S> Function() fn, {Future<Result<S>> Funct
       }),
       ClientException() => Result.failure(const NoInternetError()),
       CheckedFromJsonException() => Result.failure(const ServerError()),
+      JsonConverterException() => Result.failure(const ServerError()),
       _ => Result.failure(const UnknownError()),
     };
   }

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
 

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<TestModel, TestModel>(response), throwsA(isA<JsonUnsupportedObjectError>()));
+    await expectLater(
+      converter.convertResponse<TestModel, TestModel>(response),
+      throwsA(isA<JsonConverterException>()),
+    );
+
+    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<JsonConverterException>());
   });
 
-  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<TestModel, TestModel>(response), throwsA(isA<JsonUnsupportedObjectError>()));
+    await expectLater(
+      converter.convertResponse<TestModel, TestModel>(response),
+      throwsA(isA<JsonConverterException>()),
+    );
+
+    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<JsonConverterException>());
+  });
+
+  test('CheckedFromJsonException is logged and rethrown', () async {
+    final response = Response(baseResponse, '{"name":"John"}');
+
+    const converter = JsonSerializableConverter({ThrowingModel: ThrowingModel.fromJson});
+
+    await expectLater(
+      converter.convertResponse<ThrowingModel, ThrowingModel>(response),
+      throwsA(isA<CheckedFromJsonException>()),
+    );
+
+    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<CheckedFromJsonException>());
+    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<TestModel, TestModel>(response);
 
     expect(result.body?.name, 'John');
+    expect(memoryCharge.items, isEmpty);
   });
 
   test('Uses model to convert json into List<model>', () 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<NoContent, NoContent>(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<NoContent, NoContent>(response);
+
+    expect(result.body, {'name': 'John'});
+  });
 }
 
 class TestModel {
@@ -65,3 +131,8 @@ class TestModel2 {
 
   Map<String, dynamic> toJson() => {'name': name};
 }
+
+class ThrowingModel {
+  factory ThrowingModel.fromJson(Map<String, dynamic> json) =>
+      throw CheckedFromJsonException(json, 'name', 'ThrowingModel', 'invalid');
+}

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);