fix(client): Throw a ClientException on non-json responses, to be

 consistent with other malformed responses.
 Fix tests to also expect a ClientException in theses cases.
 Fixes zino-hofmann#552.

Author: Martin Kamleithner

packages/graphql/lib/src/link/http/link_http.dart

@@ -21,7 +21,6 @@ class HttpLink extends Link {
   HttpLink({
     @required String uri,
     bool includeExtensions,
-    bool useGETForQueries = false,
 
     /// pass on customized httpClient, especially handy for mocking and testing
     Client httpClient,
@@ -49,7 +48,6 @@ class HttpLink extends Link {
             final HttpConfig linkConfig = HttpConfig(
               http: HttpQueryOptions(
                 includeExtensions: includeExtensions,
-                useGETForQueries: useGETForQueries,
               ),
               options: fetchOptions,
               credentials: credentials,
@@ -60,42 +58,44 @@ class HttpLink extends Link {
             HttpConfig contextConfig;
 
             if (context != null) {
-              // TODO: for backwards-compatability fallback to overall context for http options
-              dynamic httpContext = context['http'] ?? context ?? {};
               // TODO: refactor context to use a [HttpConfig] object to avoid dynamic types
               contextConfig = HttpConfig(
                 http: HttpQueryOptions(
-                  includeQuery: httpContext['includeQuery'] as bool,
-                  includeExtensions: httpContext['includeExtensions'] as bool,
-                  useGETForQueries: httpContext['useGETForQueries'] as bool,
+                  includeExtensions: context['includeExtensions'] as bool,
                 ),
                 options: context['fetchOptions'] as Map<String, dynamic>,
                 credentials: context['credentials'] as Map<String, dynamic>,
                 headers: context['headers'] as Map<String, String>,
               );
             }
 
-            final HttpConfig config = _mergeHttpConfigs(
+            final HttpHeadersAndBody httpHeadersAndBody =
+                _selectHttpOptionsAndBody(
+              operation,
               fallbackHttpConfig,
               linkConfig,
               contextConfig,
             );
 
+            final Map<String, String> httpHeaders = httpHeadersAndBody.headers;
+
             StreamController<FetchResult> controller;
 
             Future<void> onListen() async {
               StreamedResponse response;
 
               try {
                 // httpOptionsAndBody.body as String
-                final BaseRequest request = await _prepareRequest(parsedUri, operation, config);
+                final BaseRequest request = await _prepareRequest(
+                    parsedUri, httpHeadersAndBody.body, httpHeaders);
 
                 response = await fetcher.send(request);
 
                 operation.setContext(<String, StreamedResponse>{
                   'response': response,
                 });
-                final FetchResult parsedResponse = await _parseResponse(response);
+                final FetchResult parsedResponse =
+                    await _parseResponse(response);
 
                 controller.add(parsedResponse);
               } catch (failure) {
@@ -163,31 +163,18 @@ Future<Map<String, MultipartFile>> _getFileMap(
 
 Future<BaseRequest> _prepareRequest(
   Uri uri,
-  Operation operation,
-  HttpConfig config,
+  Map<String, dynamic> body,
+  Map<String, String> httpHeaders,
 ) async {
-  final httpHeaders = config.headers;
-  final body = _buildBody(operation, config);
-
   final Map<String, MultipartFile> fileMap = await _getFileMap(body);
   if (fileMap.isEmpty) {
-    if (operation.isQuery && config.http.useGETForQueries) {
-      config.options['method'] = 'GET';
-    }
-
-    final httpMethod = config.options['method']?.toString()?.toUpperCase() ?? 'POST';
-    if (httpMethod == 'GET') {
-      uri = uri.replace(queryParameters: body.map((k, v) => MapEntry(k, v is String ? v : json.encode(v))));
-    }
-    final Request r = Request(httpMethod, uri);
+    final Request r = Request('post', uri);
     r.headers.addAll(httpHeaders);
-    if (httpMethod != 'GET') {
-      r.body = json.encode(body);
-    }
+    r.body = json.encode(body);
     return r;
   }
 
-  final MultipartRequest r = MultipartRequest('POST', uri);
+  final MultipartRequest r = MultipartRequest('post', uri);
   r.headers.addAll(httpHeaders);
   r.fields['operations'] = json.encode(body, toEncodable: (dynamic object) {
     if (object is MultipartFile) {
@@ -230,67 +217,97 @@ Future<BaseRequest> _prepareRequest(
   return r;
 }
 
-HttpConfig _mergeHttpConfigs(
+HttpHeadersAndBody _selectHttpOptionsAndBody(
+  Operation operation,
   HttpConfig fallbackConfig, [
   HttpConfig linkConfig,
   HttpConfig contextConfig,
 ]) {
+  final Map<String, dynamic> options = <String, dynamic>{
+    'headers': <String, String>{},
+    'credentials': <String, dynamic>{},
+  };
+  final HttpQueryOptions http = HttpQueryOptions();
+
   // http options
-  final HttpQueryOptions httpQueryOptions = HttpQueryOptions();
 
   // initialize with fallback http options
-  httpQueryOptions.addAll(fallbackConfig.http);
+  http.addAll(fallbackConfig.http);
 
   // inject the configured http options
   if (linkConfig.http != null) {
-    httpQueryOptions.addAll(linkConfig.http);
+    http.addAll(linkConfig.http);
   }
 
   // override with context http options
   if (contextConfig.http != null) {
-    httpQueryOptions.addAll(contextConfig.http);
+    http.addAll(contextConfig.http);
   }
 
-  return HttpConfig(
-    http: httpQueryOptions,
-    options: {
-      ...fallbackConfig.options,
-      ...(linkConfig != null ? linkConfig.options ?? {} : {}),
-      ...(contextConfig != null ? contextConfig.options ?? {} : {}),
-    },
-    credentials: {
-      ...fallbackConfig.credentials,
-      ...(linkConfig != null ? linkConfig.credentials ?? {} : {}),
-      ...(contextConfig != null ? contextConfig.credentials ?? {} : {}),
-    },
-    headers: {
-      ...fallbackConfig.headers,
-      ...(linkConfig != null ? linkConfig.headers ?? {} : {}),
-      ...(contextConfig != null ? contextConfig.headers ?? {} : {}),
-    },
-  );
-}
+  // options
+
+  // initialize with fallback options
+  options.addAll(fallbackConfig.options);
+
+  // inject the configured options
+  if (linkConfig.options != null) {
+    options.addAll(linkConfig.options);
+  }
+
+  // override with context options
+  if (contextConfig.options != null) {
+    options.addAll(contextConfig.options);
+  }
+
+  // headers
+
+  // initialze with fallback headers
+  options['headers'].addAll(fallbackConfig.headers);
+
+  // inject the configured headers
+  if (linkConfig.headers != null) {
+    options['headers'].addAll(linkConfig.headers);
+  }
+
+  // inject the context headers
+  if (contextConfig.headers != null) {
+    options['headers'].addAll(contextConfig.headers);
+  }
+
+  // credentials
+
+  // initialze with fallback credentials
+  options['credentials'].addAll(fallbackConfig.credentials);
+
+  // inject the configured credentials
+  if (linkConfig.credentials != null) {
+    options['credentials'].addAll(linkConfig.credentials);
+  }
+
+  // inject the context credentials
+  if (contextConfig.credentials != null) {
+    options['credentials'].addAll(contextConfig.credentials);
+  }
 
-Map<String, dynamic> _buildBody(
-  Operation operation,
-  HttpConfig config,
-) {
   // the body depends on the http options
   final Map<String, dynamic> body = <String, dynamic>{
     'operationName': operation.operationName,
     'variables': operation.variables,
-  }; 
+  };
 
   // not sending the query (i.e persisted queries)
-  if (config.http.includeExtensions) {
+  if (http.includeExtensions) {
     body['extensions'] = operation.extensions;
   }
 
-  if (config.http.includeQuery) {
+  if (http.includeQuery) {
     body['query'] = printNode(operation.documentNode);
   }
 
-  return body;
+  return HttpHeadersAndBody(
+    headers: options['headers'] as Map<String, String>,
+    body: body,
+  );
 }
 
 Future<FetchResult> _parseResponse(StreamedResponse response) async {
@@ -301,11 +318,13 @@ Future<FetchResult> _parseResponse(StreamedResponse response) async {
   final Uint8List responseByte = await response.stream.toBytes();
   final String decodedBody = encoding.decode(responseByte);
 
-  final Map<String, dynamic> jsonResponse =
-      json.decode(decodedBody) as Map<String, dynamic>;
-  final FetchResult fetchResult = FetchResult(
-    statusCode: statusCode,
-  );
+  Map<String, dynamic> jsonResponse;
+  try {
+      jsonResponse=  json.decode(decodedBody) as Map<String, dynamic>;
+  }catch(e){
+    throw ClientException('Invalid response body: $decodedBody');
+  }
+  final FetchResult fetchResult = FetchResult();
 
   if (jsonResponse['errors'] != null) {
     fetchResult.errors =

packages/graphql/test/link/http/link_http_test.dart

@@ -610,12 +610,12 @@ void main() {
 
       expect(
         exception,
-        const TypeMatcher<UnhandledFailureWrapper>(),
+        const TypeMatcher<ClientException>(),
       );
 
       expect(
-        (exception as UnhandledFailureWrapper).failure,
-        const TypeMatcher<FormatException>(),
+        (exception as ClientException).message,
+        "Invalid response body: ",
       );
     });