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

Sync up issue for iOS with nullable field and enabled secondary index #306

Closed
wer32 opened this issue Jan 13, 2021 · 8 comments
Closed

Sync up issue for iOS with nullable field and enabled secondary index #306

wer32 opened this issue Jan 13, 2021 · 8 comments
Assignees
Labels
bug Something is not working; the issue has reproducible steps and has been reproduced datastore Issues related to the DataStore Category requires-cli-fix Issues that require a CLI fix to resolve the root cause of issue.

Comments

@wer32
Copy link

wer32 commented Jan 13, 2021

Describe the bug
DataStore sync up doesn't work with iOS if the field is null in case of a secondary index is created.

type Task
@model
@auth(rules: [{ allow: owner }])
@key(name: "parent-id-index", fields: ["parentID"])
{
  id: ID!
  name: String
  status: EntityStatus!
  description: String
  parentID: ID <-- this field cause the issue
  priority: Priority
  rank: Float
  favorite: Boolean
  scheduledDateTime: AWSDateTime
  completedDateTime: AWSDateTime
  #    tags: [TaskTag] @connection(keyName: "task-tag-index", fields: ["id"])
}

in case if you send a Task with parentID field with some values the sync up works. If you send null it won't work and requires a restart of an application. I was able to reproduce the issue on iOS devices only.

To Reproduce

  1. unpack the archive
  2. setup the environment from scripts in the archive
  3. change the email to your own in the main.dart
  4. run iOS application
  5. register user
  6. login
  7. press the record button
  8. check DynamoDB table. The record must be in the table.
  9. comment line and run application again
  10. check that record is not in the table

Expected behavior

  • the record in the dynamodb table even it it is null
  • in case the internal logic is broken I would like to see an exception instead of playing a few days trying to catch the issue

Platform
Amplify Flutter currently supports iOS and Android. This issue is reproducible in (check all that apply):
[] Android
[v] iOS

Smartphone (please complete the following information):

  • Device: [e.g. iPhone 12 mini]
  • OS: [e.g. iOS14.3]

test_model_amplify.zip

@fjnoyp
Copy link
Contributor

fjnoyp commented Jan 20, 2021

Hi @wer32 thanks for your detailed explanation and the issue.

Could you clarify : If you send null it won't work and requires a restart of an application.

From what I read it sounds like the item is not being saved, but you're not getting any exception at all. Is that the case or is the application freezing and becoming unresponsive? Why do you have to restart the application?

@fjnoyp fjnoyp added bug Something is not working; the issue has reproducible steps and has been reproduced clarification-needed datastore Issues related to the DataStore Category labels Jan 21, 2021
@wer32
Copy link
Author

wer32 commented Jan 21, 2021

Hi @fjnoyp
When I send a Task with parentID = null it was saved in local DataStore but wasn't pushed to DynamoDB.
UI and etc wasn't frozen and all worked as expected except data pushing.
Please not if remove the index @key(name: "parent-id-index", fields: ["parentID"]) the syncup will work with both null and non null values.
If you see reproduced steps you can notice that I send Task with nonnull parentID field and then commented line of the code and sent one more time. And the logic broke only on the second step. Uncommenting back didn't help and sync up didn't work even with the nonnull field. I have to re-run the application in order to restore normal behavior.
Hope that will help.

@fjnoyp fjnoyp self-assigned this Feb 17, 2021
@b-cancel
Copy link

This is a bug report I just filed that includes a couple of bugs I found and the workarounds to each
after reading your bug report I think it could help
#822

@HuiSF
Copy link
Member

HuiSF commented Aug 24, 2021

Hey @wer32 I'm sorry for not keeping in touch.
I got time today to dig into this issue as I've seen some similar issues popped up recently.

Following your description, I created a minimal example to reproduce the issue.

Schema that I was testing with:

type TestModel @model @key(name: "byParentId", field: ["parentId"]) {
  id: ID!
  content: String!
  parentId: ID
}

In Dart

final testModel = TestModel(content: "Testing string");
await Amplify.DataStore.save(testModel);
print("saved")

Interestingly, there is no error happened, and Dart printed "saved" in the console. Then I inspected local SQLite DB, the model is saved, however, model is not synced to cloud.

Tried to save model again, and inspected network traffic
GraphQL request

{
	"query": "mutation CreateTestModel($input: CreateTestModelInput!) {\n  createTestModel(input: $input) {\n    id\n    content\n    parentId\n    __typename\n    _version\n    _deleted\n    _lastChangedAt\n  }\n}",
	"variables": {
		"input": {
			"content": "Test content",
			"id": "902cd218-799e-4b66-9d24-20a0d7406d22",
			"parentId": null
		}
	}
}

Response

{
	"data": {
		"createTestModel": null
	},
	"errors": [{
		"path": ["createTestModel"],
		"data": null,
		"errorType": "DynamoDB:DynamoDbException",
		"errorInfo": null,
		"locations": [{
			"line": 2,
			"column": 3,
			"sourceName": null
		}],
		"message": "One or more parameter values were invalid: Type mismatch for Index Key parentId Expected: S Actual: NULL IndexName: parent-id-index (Service: DynamoDb, Status Code: 400, Request ID: B3AHK1Q1ESMHO7635VKKGQHP9JVV4KQNSO5AEMVJF66Q9ASUAAJG, Extended Request ID: null)"
	}]
}

There is an error returned from DynamoDB

One or more parameter values were invalid:
  Type mismatch for Index Key parentId Expected: S Actual:
    NULL IndexName: parent-id-index (
      Service: DynamoDb, Status Code: 400, Request ID: 
         B3AHK1Q1ESMHO7635VKKGQHP9JVV4KQNSO5AEMVJF66Q9ASUAAJG, Extended Request ID: null
    )

It looks like that DynamoDB rejected the null value for parentId field as the parentId field is used as partition key.

At this point we have had two issues:

  1. amplify-ios didn't throw any error on this GraphQL error, hence amplify-flutter didn't forward any error either
  2. amplify-android dispatched an OutboxMutationFailedEvent, amplify-flutter, however, ignored this error (the actual error seems not helpful though as it doesn't include the actual cause)
the `OutboxMutationFailedEvent`
OutboxMutationFailedEvent{errorType=UNKNOWN, operation=CREATE, modelName=TestModel, model=SerializedModel{id='bfdb8f08-9d41-469b-808c-b2c4e559c739', serializedData={id=bfdb8f08-9d41-469b-808c-b2c4e559c739, content=Test content, parentId=null}, modelName=TestModel}}

In the GraphQL request it explicitly sent parentId: null. After removing parentId: null from variables, the GraphQL succeeded, and model is synced into DynamoDB. Hence, if a field is used as partition key, its value cannot be null, but can be empty.

Therefore, the 3rd issue is that when a field is used as partition key, when generating GraphQL document for DataStore cloud syncing, and if the value of this field is null, it should be omitted from the GraphQL document.

I'll create issues in each library repo for tracking the fix.

@HuiSF
Copy link
Member

HuiSF commented Feb 11, 2022

This is no longer an issue with GraphQL Transformer v2
example schema

type SecondIndexModel @model {
  id: ID! @primaryKey
  parentID: ID @index(name: "parent-id-index")
}

When parentID is unset, cloud sync also worked.

@HuiSF HuiSF closed this as completed Feb 15, 2022
@HuiSF
Copy link
Member

HuiSF commented Feb 23, 2022

The testing env might have been setup wrongly. This issue is still reproducible with transformer v2

@HuiSF HuiSF reopened this Feb 23, 2022
@HuiSF
Copy link
Member

HuiSF commented Mar 4, 2022

Amplify CLI opened an issue for looking for a solution aws-amplify/amplify-cli#9915

@HuiSF HuiSF added requires-cli-fix Issues that require a CLI fix to resolve the root cause of issue. and removed requires-android-fix This issue is the result of an underlying Amplify Android issue that needs to be fixed. requires-ios-fix This issue is the result of an underlying Amplify iOS issue that needs to be fixed. labels Mar 10, 2022
@HuiSF
Copy link
Member

HuiSF commented May 12, 2022

The underlying issue has been fixed on Amplify CLI side.

Please ensure upgrade to Amplify CLI version >=8.0.2, and re-deploy schema (regenerate resolvers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working; the issue has reproducible steps and has been reproduced datastore Issues related to the DataStore Category requires-cli-fix Issues that require a CLI fix to resolve the root cause of issue.
Projects
None yet
Development

No branches or pull requests

4 participants