chore(api,core): change API types #2148
Conversation
| @@ -131,8 +131,12 @@ abstract class AWSBaseHttpClient extends AWSCustomHttpClient { | |||
| responseProgressCompleter.setSourceStream(operation.responseProgress); | |||
| operation.operation.then( | |||
| (resp) async { | |||
| resp = await transformResponse(resp); | |||
| completer.complete(resp); | |||
| try { | |||
There was a problem hiding this comment.
@dnys1 double-checking this change makes sense? Without this, exceptions thrown in transformResponse weren't getting caught as expecting when await operation .response.
ragingsquirrel3
changed the title
packages/api/amplify_api/lib/src/decorators/web_socket_auth_utils.dart
Outdated
Show resolved
Hide resolved
| try { | ||
| resp = await transformResponse(resp); | ||
| completer.complete(resp); | ||
| } on Exception catch (e, st) { |
There was a problem hiding this comment.
Yeah that's fine, but let's add some tests since I didn't understand that about CancelableOperation.then
Also, let's handle all errors and exceptions this way so that errors don't get lost either.
| } on Exception catch (e, st) { | |
| } on Object catch (e, st) { |
There was a problem hiding this comment.
Made this change, plus added some unit tests in aws_common to test this behavior in request/response transform (though the REST tests in API category would fail as well without this change).
| ) async { | ||
| // For REST endpoints, throw [RestException] on non-successful responses. | ||
| if (endpointConfig.endpointType == EndpointType.rest && | ||
| (response.statusCode < 200 || response.statusCode >= 300)) { |
There was a problem hiding this comment.
Are we throwing for 3xx?
There was a problem hiding this comment.
Yes. 1) to be consistent w current behavior 2) though I guess you could make the case for following redirects that's not a change to behavior that was in API review.
There was a problem hiding this comment.
Got it. Did not know that was the current behavior.
Codecov Report
@@ Coverage Diff @@
## feat/api-next #2148 +/- ##
=================================================
+ Coverage 38.97% 39.10% +0.12%
=================================================
Files 118 119 +1
Lines 6879 6900 +21
=================================================
+ Hits 2681 2698 +17
- Misses 4198 4202 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This PR makes some changes to API category user-facing API (hate saying that) now that API review is finished. In summary:
GraphQLOperationfor query/mutate but based onCancelableOperationRestOperationwith a factory to create fromAWSHttpOperationand ensure.responsenot streamed typeRestExceptionand throws in REST methods when response not successful.Also changes tests and makes a little change to
AWSHttpClientto properly handle exceptions duringtransformResponse.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.