Amplify-js: [Bug? DX issue?] DataStore opaquely coerces my NonNull field to Null and silently fails without error

Created on 6 Nov 2020  ·  7Comments  ·  Source: aws-amplify/amplify-js

Describe the bug


My schema.graphql:

type Board @model @auth(rules: [{ allow: owner }]) {
  id: ID!
  name: String!
  lists: [List] @connection(keyName: "byBoard", fields: ["id"])
}

type List
  @model
  @key(name: "byBoard", fields: ["boardID"], queryField: "listsByBoard")
  @auth(rules: [{ allow: owner }]) {
  id: ID!
  title: String!
  boardID: ID!
  board: Board @connection(fields: ["boardID"])
  cards: [Card] @connection(keyName: "byList", fields: ["id"])
}

type Card
  @model
  @key(name: "byList", fields: ["listID", "content"], queryField: "cardsByList")
  @auth(rules: [{ allow: owner }]) {
  id: ID!
  listID: ID!
  list: List @connection(fields: ["listID"])
  content: String!
}

Here is what i am trying to do:

    const newCard = {
      id: '23',
      content: cardFormState.content,
      listID: listId, // i have checked that this is not null
    }
    await DataStore.save(new Card(newCard))

Yet when I run this code it does not error, does not save this to AppSync/DynamoDB, and logs this warning in the browser:

"Variable 'input' has coerced Null value for NonNull type 'ID!'"

the full response is {"data":null,"errors":[{"path":null,"locations":[{"line":1,"column":20,"sourceName":null}],"message":"Variable 'input' has coerced Null value for NonNull type 'ID!'"}]}

I assume this is just graphql helpfully suppressing the error for us. I would expect DataStore to surface this error.

However even this error message is not helpful, and I have no idea why DataStore is coercing what i have clearly checked is a string, to null. I am out of ideas on how to fix this.

To Reproduce

you can clone https://github.com/sw-yx/trello-amplified and add auth and add api (my attempt here)

or

you can watch my 2 minute video bug report here: https://youtu.be/25PWIBfhfZE

Expected behavior

  1. it should error when there is an error
  2. error message should give more information than this
  3. why is it even coercing null value for my non null input

context

DataStore to-be-reproduced

Most helpful comment

  1. I confirmed with @iartemiev that (1) is working is working "as intended" for now. Datastore does not auto populate connections you'll have to query by connected id for now as seen in my repo.

  2. One reason is that we don't want to throw errors when one model fails to sync; so others continue syncing to minimize data loss. Definitely still worth more discussions.




Now, why did the model pass local validations validateModelFields before sending a appsync graphql request?

boardID: ID! = null passed because it is missing a field definition in generated models/schema.js:

Snippet from schema.graphql:

type List
  @model
  @key(name: "byBoard", fields: ["boardID"], queryField: "listsByBoard")
  @auth(rules: [{ allow: owner }]) {
  id: ID!
  title: String!
  boardID: ID!
  board: Board @connection(fields: ["boardID"])
  cards: [Card] @connection(keyName: "byList", fields: ["id"])
}

Snippet from schema.js missing boardID field definition:

"List": {
    "name": "List",
    "fields": {
        "id": {
            "name": "id",
            "isArray": false,
            "type": "ID",
            "isRequired": true,
            "attributes": []
        },
        "title": {
            "name": "title",
            "isArray": false,
            "type": "String",
            "isRequired": true,
            "attributes": []
        },
        "board": {
            "name": "board",
            "isArray": false,
            "type": {
                "model": "Board"
            },
            "isRequired": false,
            "attributes": [],
            "association": {
                "connectionType": "BELONGS_TO",
                "targetName": "boardID"
            }
        },
        "cards": {
            "name": "cards",
            "isArray": true,
            "type": {
                "model": "Card"
            },
            "isRequired": false,
            "attributes": [],
            "isArrayNullable": true,
            "association": {
                "connectionType": "HAS_MANY",
                "associatedWith": "list"
            }
        },
        "owner": {
            "name": "owner",
            "isArray": false,
            "type": "String",
            "isRequired": false,
            "attributes": []
        }
    },
    "syncable": true,
    "pluralName": "Lists",
    "attributes": [
        {
            "type": "model",
            "properties": {}
        },
        {
            "type": "key",
            "properties": {
                "name": "byBoard",
                "fields": [
                    "boardID"
                ],
                "queryField": "listsByBoard"
            }
        },
        {
            "type": "auth",
            "properties": {
                "rules": [
                    {
                        "provider": "userPools",
                        "ownerField": "owner",
                        "allow": "owner",
                        "identityClaim": "cognito:username",
                        "operations": [
                            "create",
                            "update",
                            "delete",
                            "read"
                        ]
                    }
                ]
            }
        }
    ]
},

So the code snippet in my first comment is setting boardID to null and then the local validation is passing it without checks as the boardID field definition is undefined.

All 7 comments

I can reproduce.

I believe this is the exact same issue as mentioned in https://github.com/aws-amplify/amplify-js/issues/5161 However, that issue does not seem to have a clear solution posted.


With your code, I can reproduce the same "Variable 'input' has coerced Null value for NonNull type 'ID!'" error when adding a list and it can be traced to:

https://github.com/aws-amplify/amplify-js/blob/5173a9911096627ce1b45067808af249668b260b/packages/datastore/src/util.ts#L235-L268

Relevant files:

Since we only set list.boardID field, rItem.fieldName (board) which is undefined. The logic on L260-264 is setting rItem.targetName (boardID) to null when list.board is falsey. This results in datastore sending a null list.boardID in the graphql request even though it was provided correctly.

(I don't understand what L236-258 is doing though)

I think you are expecting datastore to populate list.board (or card.list) automatically by just specifying list.boardID (or card.listID) Not sure if we need to manually set list.board = board manually as it doesn't look like something datastore is doing automatically❓


Anyways, after reviewing graphql docs and datastore docs. I'm not sure if your expected behavior above is supported, it seems like the relational modeling has limited support in datastore compared to GraphQL. https://github.com/aws-amplify/amplify-js/issues/5054#issuecomment-598868471 also shows certain operations are just not yet possible with datastore at this time.

I've made some modifications strictly following the datastore docs example and it's now functional. The repo is at wei/amplify-js-7127 and the commit can be seen at https://github.com/wei/amplify-js-7127/commit/f03feacfd595be84c93b7de6936a0b2292dcad93

Summary of my changes in the commit:

  • Removed the the list.board @connection
  • Fetch cards using await DataStore.query(CardModel)
  • Pass in cards to List component differently, like this

Lastly, going back to the original question, this is the piece of code that runs on our error:
https://github.com/aws-amplify/amplify-js/blob/5173a9911096627ce1b45067808af249668b260b/packages/datastore/src/datastore/datastore.ts#L422-L424

and it looks like:
image

I agree it's not immediately apparent that there has been an error, perhaps as an easy first step, we can change it to include the word Sync error like it's already been done here:

https://github.com/aws-amplify/amplify-js/blob/5173a9911096627ce1b45067808af249668b260b/packages/datastore/src/datastore/datastore.ts#L625-L628

or a different phrase to indicate an error. Any feedback❓

/cc @iartemiev

i'm super grateful that you spent the time to repro!!!

  1. yes, i would expect DataStore to populate that for me - notice that it does so for list.board but not list.cards... thats a separate issue i ran into that was a dealbreaker for me :(
  2. i HATE silent errors with a passion. logging a warn isn't good enough for me imo, i would actually throw an error. why do we automatically downgrade errors to warnings? this is the sort of monkey business people criticize graphql for, and with datastore we are trying to provide a cleaner abstraction over graphql.

Sure thing!

RE (1): Could you start fresh with a new clone, checkout swyxDataStoreVersion branch, use a new amplify env and run amplify codegen models before amplify push, Clear the local indexdb too or use incognito.
it's not populating list.board for me. It actually throws an error on this line because list.board is undefined:
https://github.com/sw-yx/trello-amplified/blob/64f5dcd8401b6d7cd90608cbefc3d0e5b34db7f0/src/pages/Board.js#L15

RE (2): I did notice some data sync errors show up as warnings, will need to defer to datastore maintainer for the reasoning behind that.

thats a separate issue i ran into that was a dealbreaker for me :(

RE (1): Check out #6973 ; )

DataStore doesn't currently support syncing any connections to a model

I guess it's just not possible right now. For the time being you can work on top of my repo which is using what's currently possible with datastore to create trello-amplified.

appreciate the superfast feedback!! so i guess the remaining question is:

  1. is there a bug or is it working "as intended" for now?
  2. there is definitely a DX issue we could discuss around whether errors should be errors, or errors should be warnings, when using AppSync.
  1. I confirmed with @iartemiev that (1) is working is working "as intended" for now. Datastore does not auto populate connections you'll have to query by connected id for now as seen in my repo.

  2. One reason is that we don't want to throw errors when one model fails to sync; so others continue syncing to minimize data loss. Definitely still worth more discussions.




Now, why did the model pass local validations validateModelFields before sending a appsync graphql request?

boardID: ID! = null passed because it is missing a field definition in generated models/schema.js:

Snippet from schema.graphql:

type List
  @model
  @key(name: "byBoard", fields: ["boardID"], queryField: "listsByBoard")
  @auth(rules: [{ allow: owner }]) {
  id: ID!
  title: String!
  boardID: ID!
  board: Board @connection(fields: ["boardID"])
  cards: [Card] @connection(keyName: "byList", fields: ["id"])
}

Snippet from schema.js missing boardID field definition:

"List": {
    "name": "List",
    "fields": {
        "id": {
            "name": "id",
            "isArray": false,
            "type": "ID",
            "isRequired": true,
            "attributes": []
        },
        "title": {
            "name": "title",
            "isArray": false,
            "type": "String",
            "isRequired": true,
            "attributes": []
        },
        "board": {
            "name": "board",
            "isArray": false,
            "type": {
                "model": "Board"
            },
            "isRequired": false,
            "attributes": [],
            "association": {
                "connectionType": "BELONGS_TO",
                "targetName": "boardID"
            }
        },
        "cards": {
            "name": "cards",
            "isArray": true,
            "type": {
                "model": "Card"
            },
            "isRequired": false,
            "attributes": [],
            "isArrayNullable": true,
            "association": {
                "connectionType": "HAS_MANY",
                "associatedWith": "list"
            }
        },
        "owner": {
            "name": "owner",
            "isArray": false,
            "type": "String",
            "isRequired": false,
            "attributes": []
        }
    },
    "syncable": true,
    "pluralName": "Lists",
    "attributes": [
        {
            "type": "model",
            "properties": {}
        },
        {
            "type": "key",
            "properties": {
                "name": "byBoard",
                "fields": [
                    "boardID"
                ],
                "queryField": "listsByBoard"
            }
        },
        {
            "type": "auth",
            "properties": {
                "rules": [
                    {
                        "provider": "userPools",
                        "ownerField": "owner",
                        "allow": "owner",
                        "identityClaim": "cognito:username",
                        "operations": [
                            "create",
                            "update",
                            "delete",
                            "read"
                        ]
                    }
                ]
            }
        }
    ]
},

So the code snippet in my first comment is setting boardID to null and then the local validation is passing it without checks as the boardID field definition is undefined.

@mauerbac we still were at "Definitely still worth more discussions." in the previous comment

Was this page helpful?
0 / 5 - 0 ratings