Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscriptions can fail depending on source mutation return parameters #216

Closed
airstance opened this issue Apr 2, 2019 · 8 comments
Closed
Labels
feature-request New feature requests

Comments

@airstance
Copy link

My subscription handler is being called with an error.

To Reproduce
Steps to reproduce the behavior:

  1. Use the Event example
  2. Subscribe to onDeltaPost
  3. Call updatePost but only ask for id to be returned
  4. The subscription routine will be called with an error

Expected behavior
The getPost call works when only the id is requested. I would expect the same here. The subscription handler should be called with as many fields as are available.

Environment(please complete the following information):

  • AppSync SDK Version: latest
  • Dependency Manager: Cocoapods
  • Swift Version : 4.2+

Additional context
On a related note, it seems very odd that the subscription handler is only called with the result parameters for the mutation. If the mutation call only asks for minimal data, i would be forced to make a manual fetch call for the object. Even that won't work if the mutation request does not ask for all the return parameters necessary to make the manual fetch. I apologize in advance for my lack of knowledge of AppSync subscriptions.

@airstance airstance changed the title Subscriptions can fail depending on source event return parameters Subscriptions can fail depending on source mutation return parameters Apr 2, 2019
@rohandubal
Copy link
Contributor

Hello @airstance

This is a service requirement from what I can tell. The subscription can only get data which was specified in the mutation. Subscription data can be a subset of mutation selections, but not a super set. You will have to select the right fields in mutation for subscriptions to function correctly.

Thanks,
Rohan

@rohandubal rohandubal added AppSync question A question about how to use an existing feature labels Apr 4, 2019
@airstance
Copy link
Author

When publishing a GraphQL interface to outside developers, I have to ask them to make sure they include a minimum set of return parameters, otherwise, they'll cause some subscriptions to fail. That doesn't seem very robust. I understand there's nothing the iOS client can do about the response. I'll probably ask for a feature request for the AppSync service to add the ability to create response resolvers for handling subscription clients.

To work around this limitation, I first tried to re-create the GraphQL types and made all required fields optional. That would allow the client subscription validation work and at least pass me what fields were supplied. Right now I get nothing. That worked great, but then I realized this didn't really help me because I sometimes still had an error condition (missing data) that I could do nothing about.

In the end, I called the modified syncNow() method (#215) in the subscription handlers if there were missing parameters. That allows the delta catch-up sync code to catch the offending mutation changes.

@johnmurphy01
Copy link
Contributor

The original bug report is also happening to us. We have a subscription to our entity, we'll call it Post. When a Post is created, we have a server function that updates thumbnails and kicks off an updatePost mutation that only returns the id field. In the iOS client, this causes the app to crash. Our server team made a change and now return all fields in the mutation after updating thumbnails and this appears to have resolved the issue. However this is a fragile work around and hopefully there will be a fix soon.

I definitely think you should be able to return a subset of fields rather than have to return all fields(and sub selections) defined in the graph.

@johnmurphy01
Copy link
Contributor

@rohandubal I think there's a bug here. I can post any additional information necessary

@palpatim palpatim added feature-request New feature requests Service and removed question A question about how to use an existing feature labels May 9, 2019
@palpatim
Copy link
Contributor

palpatim commented May 9, 2019

I think this could be argued as buggy behavior--it's certainly unexpected--but the behavior is by design on the service. We'll ask the service team to investigate this and post a reply when we have an update.

In the meantime, we'll also investigate #215 to see if there's a mitigation on the client side that we can apply without impacting other behaviors.

@johnmurphy01
Copy link
Contributor

Here's the Crashlytics raw text if it helps:

This still appears to be happening even after adjusting the return parameters on the server, so our "workaround" doesn't appear to be viable at this point either.

Crashed: com.apple.root.user-initiated-qos
0  libswiftFoundation.dylib       0x1d592c400 specialized Data._Representation.subscript.getter + 704
1  libswiftFoundation.dylib       0x1d5861038 protocol witness for Collection.subscript.getter in conformance Data + 32
2  libswiftCore.dylib             0x1d54f45b8 Collection.prefix(upTo:) + 356
3  AWSAppSync                     0x104d55f3c $s10AWSAppSync0aB19SubscriptionWatcherC23messageCallbackDelegate4datay10Foundation4DataV_tFSSyXEfu0_ + 228
4  AWSAppSync                     0x104cf654c $s10AWSAppSync03AppB3LogC3log33_66ACA86EE1962FC325D68C680BC0BDC7LL_4flag4file8function4lineySSyXK_So12AWSDDLogFlagVS2SSitFZ + 376
5  AWSAppSync                     0x104cf63c4 $s10AWSAppSync03AppB3LogC7verbose_4file8function4lineySSyXK_S2SSitFZ + 296
6  AWSAppSync                     0x104d552ec $s10AWSAppSync0aB19SubscriptionWatcherC23messageCallbackDelegate4datay10Foundation4DataV_tF + 756
7  AWSAppSync                     0x104d56d78 $s10AWSAppSync0aB19SubscriptionWatcherC23messageCallbackDelegate4datay10Foundation4DataV_tFTo + 84
8  AWSAppSync                     0x104cfc79c $s10AWSAppSync03AppB10MQTTClientC19receivedMessageData_7onTopicy10Foundation0G0VSg_SSSgtFyyXEfU_yycfU_ + 604
9  AWSAppSync                     0x104ce440c $sIeg_IeyB_TR + 52
10 libdispatch.dylib              0x1a7460a38 _dispatch_call_block_and_release + 24
11 libdispatch.dylib              0x1a74617d4 _dispatch_client_callout + 16
12 libdispatch.dylib              0x1a7412160 _dispatch_root_queue_drain + 680
13 libdispatch.dylib              0x1a74128d0 _dispatch_worker_thread2 + 128
14 libsystem_pthread.dylib        0x1a76411b4 _pthread_wqthread + 464
15 libsystem_pthread.dylib        0x1a7643cd4 start_wqthread + 4

@royjit
Copy link
Contributor

royjit commented Aug 6, 2019

@airstance Did this PR #259 fix your issue?

@royjit royjit added requesting info Further information is needed before this is actionable feature-request New feature requests Service and removed feature-request New feature requests Service labels Aug 6, 2019
@palpatim palpatim removed the AppSync label Nov 4, 2020
@palpatim
Copy link
Contributor

palpatim commented Nov 4, 2020

Closing old issue.

There's not a good workaround here--the strongly-typed return values expect values in required fields, and if those values aren't in the mutation's selection set, then the subscription won't have them and will deliver a partial result. In Amplify DataStore, we carefully craft mutations with fully-populated selection sets so that subscriptions are delivered as expected. As any change to this behavior would need to be handled on the service, so I'm closing this client-side issue.

@palpatim palpatim closed this as completed Nov 4, 2020
@palpatim palpatim removed requesting info Further information is needed before this is actionable Service labels Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature requests
Projects
None yet
Development

No branches or pull requests

5 participants