Changing "with_base_url" from true to false changes the default Json converter and breaks deserialization

[point-source]

EDIT: There is a more accurate identification of this issue in this comment.

It appears that the JsonSerializableConverter() was removed in 2.0.8+1 as part of this PR and as documented in more detail on this comment.

Using chopper.JsonConverter() in its place does not appear to work as this error is produced for most if not all requests (CrewGetResponse shown here for example purposes):

Unhandled exception:
type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'CrewGet$Response?'
#0      JsonConverter.decodeJson (package:chopper/src/interceptor.dart:274:46)
#1      JsonConverter.convertResponse (package:chopper/src/interceptor.dart:279:12)
#2      ChopperClient._decodeResponse (package:chopper/src/base.dart:186:29)
#3      ChopperClient._handleSuccessResponse (package:chopper/src/base.dart:260:17)
#4      ChopperClient.send (package:chopper/src/base.dart:322:19)

Manually restoring the original JsonSerializableConverter from a previous commit and using it in the chopper client appears to fix this issue. Hopefully, there is a better fix than that though.

[Vovanella95]

Hi @point-source, the issue is about JsonSerializableConverter is not generated? Can you provide concrete json file and point on expected and actual retuls?

[Vovanella95]

@point-source and please share build.yaml to restore all picture

[point-source]

Ah, good catch. Turns out it was not a version/PR issue. I had changed the build.yaml around the time I moved from 2.0.7 to 2.0.8 so I incorrectly assumed it was related to the version change.

That said, the option I changed was with_base_url: true to with_base_url: false and since that seemed very straightforward, I did not expect it to have this side-effect. Here is a snippet of what the lasso.swagger.dart file looks like when with_base_url is set to true:

// **************************************************************************
// SwaggerChopperGenerator
// **************************************************************************

@ChopperApi()
abstract class Lasso extends ChopperService {
  static Lasso create([ChopperClient? client]) {
    if (client != null) {
      return _$Lasso(client);
    }

    final newClient = ChopperClient(
        services: [_$Lasso()],
        converter: JsonSerializableConverter(),
        baseUrl: 'https://yourcompany.lasso.io/api/v1');
    return _$Lasso(newClient);
  }

And when set to false:

// **************************************************************************
// SwaggerChopperGenerator
// **************************************************************************

@ChopperApi()
abstract class Lasso extends ChopperService {
  static Lasso create([ChopperClient? client]) {
    if (client != null) {
      return _$Lasso(client);
    }

    final newClient = ChopperClient(
      services: [_$Lasso()],
      converter: chopper.JsonConverter(), /*baseUrl: YOUR_BASE_URL*/
    );
    return _$Lasso(newClient);
  }

Notice that converter: JsonSerializableConverter(), has changed to converter: chopper.JsonConverter(),

Is this the expected behavior? The chopper.JsonConverter() version does not correctly parse the server response in my experience so far. If this is the intended behavior, we should at least document it as part of the with_base_url option docs.

[Vovanella95]

Hi @point-source , yes, it's expected behavior. But it's not logical. Just legacy decision.

I've fixed it in 2.1.0 branch.

Now there are two properties with_base_url and with_converter you can use. Please try it and let me know if everything OK :)

[Vovanella95]

@point-source , also you can already use pub version 2.1.0-prerelease.2

[point-source]

2.1.0-prerelease.2 does appear to be including the right json converter, so thanks for that. However, it also seems to be changing the way methods and types are named and this is breaking generation on my lasso package/swagger.

Specifically, this:

  ///
  ///@param external_code
  ///@param name
  ///@param slug
  ///@param limit Number of results to return per page.
  ///@param offset The initial index from which to return the results.

  @Get(path: '/account_event_statuses')
  Future<chopper.Response<AccountEventStatusesGet$Response>>
      accountEventStatusesList(
          {@Query('external_code') String? external_code,
          @Query('name') String? name,
          @Query('slug') String? slug,
          @Query('limit') int? limit,
          @Query('offset') int? offset});

has changed to this:

  ///
  ///@param external_code
  ///@param name
  ///@param slug
  ///@param limit Number of results to return per page.
  ///@param offset The initial index from which to return the results.
  @Get(path: '/account_event_statuses')
  Future<chopper.Response<AccountEventStatusesList$Response>>
      account_event_statuses_list(
          {@Query('external_code') String? external_code,
          @Query('name') String? name,
          @Query('slug') String? slug,
          @Query('limit') int? limit,
          @Query('offset') int? offset});

and the new code does not follow dart format conventions so the analyzer warns on the snake-case method. Additionally, the new type name AccountEventStatusesList$Response does not resolve as the definition for it is still named AccountEventStatusesGet$Response.

Since these regressions are off-topic for this issue, would you like me to open a new issue for them?

[Vovanella95]

Hi @point-source ,

Sure open new issue I will look at it

[Vovanella95]

Currently I've focused to release 2.1.0 with fully reimplemented requests generator. So all fixes will be applied to 2.1.0 version. Let's make it perfect :)

[point-source]

Sounds good to me. Closing since this is now resolved via the new with_converter option.