Skip to content

Commit

Permalink
fix(client): Throw a ClientException on non-json responses, to be
Browse files Browse the repository at this point in the history 
 consistent with other malformed responses.
 Fix tests to also expect a ClientException in theses cases.
 Fixes #552.
  • Loading branch information
@micimize
Martin Kamleithner authored and micimize committed Jul 24, 2020
1 parent b22d911 commit 060eff2
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 68 deletions.
149 changes: 84 additions & 65 deletions packages/graphql/lib/src/link/http/link_http.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -49,7 +48,6 @@ class HttpLink extends Link {
final HttpConfig linkConfig = HttpConfig(
http: HttpQueryOptions(
includeExtensions: includeExtensions,
useGETForQueries: useGETForQueries,
),
options: fetchOptions,
credentials: credentials,
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 =
Expand Down
6 changes: 3 additions & 3 deletions packages/graphql/test/link/http/link_http_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: ",
);
});

Expand Down

0 comments on commit 060eff2

Please sign in to comment.