Graphql-flutter: Data from partially successful queries is not returned

[fusion44]

Describe the bug
If a named query contains an error, data from successfully executed subqueries is simply discarded.

I have a similar query to this:

query getStats {
  statusA: getSystemStatusA {
    uptime
    cpuLoad
  }
  statusB: getSystemStatusB {
    uptime
    cpuLoad
  }
  statusC: getSystemStatusC {
    uptime
    cpuLoad
  }
}

which returns this result:

{
  "errors": [
    {
      "message": "error message",
      "locations": [
        {
          "line": 10,
          "column": 3
        }
      ],
      "path": [
        "statusB"
      ]
    },
  ],
  "data": {
    "statusA": {
      "uptime": 123,
      "cpuLoad": 6.25,
    },
    "statusB": null,
    "statusC": {
      "uptime": 456,
      "cpuLoad": 6.25,
    }
  }
}

Expected behavior
I'd expect to receive the errors and the data without an exception. I'd only expect an exception if the http status code is not 200.

Additional context
Problem is a bit of code in client.dart[78]

Is there any special reason to drop all the data? I'd like to show an appropriate error message in my app ui for the failed queries.

My current simplistic solution is to comment out the if block and just return the whole JSON object, including all errors.

    final Map<String, dynamic> jsonResponse = json.decode(response.body);
    return jsonResponse;

    /*     
    if (jsonResponse['errors'] != null && jsonResponse['errors'].length > 0) {
      throw GQLException(
        'Error returned by the server in the query',
        jsonResponse['errors'],
      );
    } 

    return jsonResponse['data'];
    */

Thanks!

[HofmannZ]

Thanks for the well structured issue. You're actually right, the errors should be returned along side the data.

I'm currently woking on rewriting some core transport logic. Essentially splitting everything in small links just like Apollo Client does. This allow you to handle errors more gracefully and solving this issue altogether.

For now, feel free to push a patch.

[fusion44]

OK, I'll prepare a pull request. Though, this would be a breaking change. I could also rearrange the data to keep backward compatibility with the downside that it's less standard GQL. I think since you're working on a bigger breaking change, it's better to just rearrange the data until your changes are in.

[HofmannZ]

@fusion44 fixed in #36.

To try it out use version 1.0.0-alpha.3 of this package.

[fusion44]

@HofmannZ Sorry this took so long. Had to refactor my app quite a bit to adapt to 1.0.0 :-) I'm using alpha5 from pub.

As long as my server doesn't return an error, everything works very well. When I force my server to include an error, I get this:

E/flutter ( 8673): type 'List<dynamic>' is not a subtype of type 'List<Map<String, dynamic>>' in type cast
E/flutter ( 8673): #0      QueryManager.fetchQuery (package:graphql_flutter/src/core/query_manager.dart:114:19)
E/flutter ( 8673): <asynchronous suspension>
E/flutter ( 8673): #1      QueryManager.query (package:graphql_flutter/src/core/query_manager.dart:55:12)
E/flutter ( 8673): #2      GraphQLClient.query (package:graphql_flutter/src/graphql_client.dart:38:25)
E/flutter ( 8673): #3      _StatsPageState._fetchData (package:mobile_app/pages/stats.dart:78:10)
E/flutter ( 8673): <asynchronous suspension>

My query is below. I forced lnGetInfo to return an authentication error, but this could be any other error.

query getBlocks {
  systemstatus: getSystemStatus {
    uptime
    cpuLoad
    trafficIn
    trafficOut
    memoryUsed
    memoryTotal
  }
  testnetblocks: getBlockchainInfo(testnet: true) {
    blocks
  }
  mainnetblocks: getBlockchainInfo(testnet: false) {
    blocks
  }
  testnetnetwork: getNetworkInfo(testnet: true) {
    subversion
    connections
    warnings
  }
  mainnetnetwork: getNetworkInfo(testnet: false) {
    subversion
    connections
    warnings
  }
  testnetln: lnGetInfo(testnet: true) {
    alias
    blockHeight
    identityPubkey
    numActiveChannels
    numPeers
    syncedToChain
    testnet
    version
  }
  mainnetln: lnGetInfo(testnet: false) {
    alias
    blockHeight
    identityPubkey
    numActiveChannels
    numPeers
    syncedToChain
    testnet
    version
  }
}

In this case I forced lnGetInfo to return an unauthenticated error.
This is the returned data:

{
  "errors": [
    {
      "message": "{\"code\": 2, \"message\": \"Unauthenticated\"}",
      "locations": [
        {
          "line": 26,
          "column": 3
        }
      ],
      "path": [
        "testnetln"
      ]
    },
    {
      "message": "{\"code\": 2, \"message\": \"Unauthenticated\"}",
      "locations": [
        {
          "line": 36,
          "column": 3
        }
      ],
      "path": [
        "mainnetln"
      ]
    }
  ],
  "data": {
    "systemstatus": {
      "uptime": 1238299.3477287292,
      "cpuLoad": 5.5,
      "trafficIn": 3404875516.4072,
      "trafficOut": 3598500333.5470805,
      "memoryUsed": 1746718720,
      "memoryTotal": 8048193536
    },
    "testnetblocks": {
      "blocks": 1412696
    },
    "mainnetblocks": {
      "blocks": 540517
    },
    "testnetnetwork": {
      "subversion": "/Satoshi:0.16.1/",
      "connections": 11,
      "warnings": "Warning: unknown new rules activated (versionbit 28)"
    },
    "mainnetnetwork": {
      "subversion": "/Satoshi:0.16.1/",
      "connections": 24,
      "warnings": ""
    },
    "testnetln": null,
    "mainnetln": null
  }
}

[HofmannZ]

@fusion44 Yeah it's an issue with typecasting, to be precise line 202 of the LttpLink. I Already proposed a fix for some incorrect typecasting. Just add a TODO to address your problem. If you can review the PR, I might be able to release it later today.

Cheers ✌🏻

[HofmannZ]

Fixed by PR #93.

[fusion44]

@HofmannZ I think you meant PR #76 ?