ResponseConverterInterceptor overrides null body

[flodaniel]

Steps to Reproduce

1. Doing a GET request that returns "" (an empty response), I want to convert it to null, which is done by my own JsonConverter implementation:

Service Implementation:

@ChopperApi()
abstract class AppUpdateInformationService extends ChopperService {
  @Get(headers: {'content-type': 'application/json'})
  Future<Response<AppUpdateInformationResponse?>> getAppUpdateInformation(
      {@Query('appVersion') int appVersion = 0,
      @Query('platform') String platform = ''});

  static AppUpdateInformationService create(AuthenticationRepository authRepo) {
    final client = ServiceHelper.getChopperClient(
      servicePath: 'app-update-information',
      converter: JsonToTypeConverter({
        AppUpdateInformationResponse: AppUpdateInformationResponse.fromJson
      }),
      authenticator: PaveRefreshTokenAuthenticator(authRepo),
      services: [_$AppUpdateInformationService()],
    );

    return _$AppUpdateInformationService(client);
  }
}

JsonConverter

 @override
  Response<BodyType> convertResponse<BodyType, InnerType>(Response response) {
    try {
      var convertedBody = response.body;
      if (response.body is String && InnerType != String) {
        convertedBody = fromJsonData<BodyType, InnerType>(
          response.body,
          typeToJsonFactoryMap[InnerType],
        );
      }

      if (convertedBody == null) {
        return Response<BodyType>(response.base, null, error: response.error);
      }

      return response.copyWith<BodyType>(body: convertedBody);
    } catch (e, stackTrace) {
      SentryService().recordError(
        '''Failed to convert response for BodyType $BodyType and InnerType $InnerType''',
        stackTrace: stackTrace,
        stringHint: response.bodyString,
      );
      rethrow;
    }
  }

It returns Response<BodyType>(response.base, null, error: response.error);

2. The in v8 introduced ResponseConverterInterceptor ignores the null response and tries to use the "" (empty string) response on line 64:

    return Response<BodyType>(
      newResponse?.base ?? response.base,
      newResponse?.body ?? response.body,
    );

Should I use a different approach to this or is this a bug?

[techouse]

@Guldem can probably shed some light on this.

I would maybe suggest using a different approach and always returning a JSON payload with some empty state or returning an HTTP/204 (No Content) status and then intercepting that status rather than an looking for an empty string.

[Guldem]

While I agree with @techouse that I'm not sure if the approach is right. I do also thing this is a small bug.

A Response converter should be able to change the body of the response including null. I guess I have overlooked that part initially.

I have submitted a fix #623