Open duckbytes opened 2 years ago
Hey @duckbytes thanks for opening this issue. Can you confirm if you've been able to reproduce this using the reproduction steps on the fresh app you mentioned on Discord? (when the createdBy
field is required). Can you confirm that you haven't configured a conflict resolution strategy override for that particular model? Selection sets or input shouldn't have an affect on optimistic concurrency.
Finally, can you run amplify diagnose --send-report
and share the output identifier ID?
I can confirm the issue is reproducible following the steps described by @duckbytes. I'll look into possible causes of the issue.
Thanks @ncarvajalc for looking into it.
@chrisbonifacio sorry for the delay. Maybe not needed now but I generated a report for the reproduction: d6e5f7b6ca9c454ae146353bd5eba025
This is a report for my actual app where I first discovered the issue: c483e5370c77cedae471340404b9b9de but I've already updated that so createdBy is nullable to work around the issue. So it might not be as helpful.
I was able to reproduce this issue on both my main project and the fresh reproduction I was talking about.
I haven't configured any conflict resolution overrides for a particular model (didn't know that could be done! could be useful).
@duckbytes, I looked for the reason why this is happening and found the following:
First of all, I think the Amplify Docs are not clear in describing the behavior with a required @belongsTo relationship. Let me explain:
In the DataStore documentation there is a relationships tutorial which shows an updated schema to show relationships. From the model it is inferred that Comment needs to be associated with a Post to be created. Nevertheless, this is not the behavior that is taking place.
Using the following model as an example (the one that you are having the issue with), we can see that a Todo needs to have a User to be created:
type Todo @model {
id: ID!
createdBy: User! @belongsTo
name: String!
description: String
}
type User @model {
id: ID!
todos: [Todo] @hasMany
}
When the API is created and we want to create a Todo, look what happens: It shouldn't be possible to create a Todo without a user. Also, look that the relationship is not mandatory in the explorer:
The same behavior is not consistent with the @hasOne
directive.
Now let's look at this model:
type Todo @model {
id: ID!
createdBy: User! @belongsTo
name: String!
description: String
}
type User @model {
id: ID!
todos: Todo @hasOne
}
And the result when trying to create a Todo without a User: Observe that in the explorer the relationship is now mandatory:
To accomplish the previous behavior with the @hasMany
directive, the following model has to be used:
type Todo @model {
id: ID!
createdBy: User @belongsTo
name: String!
description: String
}
type User @model {
id: ID!
todos: [Todo]! @hasMany
}
Observe that the ! is placed in the Todo array ([Todo]!) Important note: It is not the same as [Todo!].
Looking at the results: And the explorer:
Notice that the relationship is now required to create a Todo.
With that in mind, I found why the warnings are being printed and not letting the custom conflictHandler
execute properly. It has to do with the use of the WeakSet
class in the modelInstanceCreator
function in datastore.ts
:
function modelInstanceCreator<T extends PersistentModel>(
modelConstructor: PersistentModelConstructor<T>,
init: Partial<T>
): T {
instancesMetadata.add(init);
return new modelConstructor(<ModelInit<T, PersistentModelMetaData<T>>>init);
}
Look what happens when I create a WeakSet and try to add a null value in it:
It is the same warning you are getting! That means that something is being null
when we call the modelInstanceCreator
function, and that is error.data
.
Take a look at the code that is being executed in mutation.ts
in the datastore package:
if (error.errorType === 'ConflictUnhandled') {
// TODO: add on ConflictConditionalCheck error query last from server
attempt++;
let retryWith: PersistentModel | typeof DISCARD;
if (attempt > MAX_ATTEMPTS) {
retryWith = DISCARD;
} else {
try {
retryWith = await this.conflictHandler({
modelConstructor,
localModel: this.modelInstanceCreator(
modelConstructor,
variables.input
),
remoteModel: this.modelInstanceCreator(
modelConstructor,
error.data // This is being null
),
operation: opType,
attempts: attempt,
});
} catch (err) {
logger.warn('conflict trycatch', err);
continue;
}
}
Observe that error.data
is what is being null
as shown in the request you showed:
{"data":{"updateTodo":null},"errors":[{"path":["updateTodo"],"data":null,"errorType":"ConflictUnhandled","errorInfo":null,"locations":[{"line":2,"column":3,"sourceName":null}],"message":"Conflict resolver rejects mutation."}]}
But why is that being null in the first place and not when you make the User optional in the schema?
Something else that I found was that the userTodosId
is undefined
when trying to upload to DataStore as shown in this image:
The same happens even when the User is not required.
My guess is that when you make the User not required, it doesn't check for the userTodosId
when updating. I'll try to look deeper into your issue, but I wanted you to know what I found in the mean time.
Using the AppSync query console I used the query generated by Datastore to try to replicate the error.data
being null
: The results where the following:
Request query when the User
is required and createdBy { id _deleted }
is used :
mutation nullErrors {
updateTodo(
input: {
description: "No",
id: "cac76cab-a71c-48f1-ae23-bd9c529195ad",
name: "F"
}
) {
id
name
description
createdAt
updatedAt
_version
_lastChangedAt
_deleted
createdBy {
id
_deleted
}
}
}
Response
{
"data": {
"updateTodo": null
},
"errors": [
{
"path": [
"updateTodo"
],
"data": null,
"errorType": "ConflictUnhandled",
"errorInfo": null,
"locations": [
{
"line": 27,
"column": 3,
"sourceName": null
}
],
"message": "Conflict resolver rejects mutation."
}
]
}
In this case, errors.data
is null
.
There is a way for making the errors not null
and it is removing the createdBy { id _deleted }
.
Request query when the User
is required without the createdBy { id _deleted }
mutation noNullErrors {
updateTodo(
input: {
description: "No",
id: "cac76cab-a71c-48f1-ae23-bd9c529195ad",
name: "F"
}
) {
id
name
description
createdAt
updatedAt
_version
_lastChangedAt
_deleted
}
}
Response
{
"data": {
"updateTodo": null
},
"errors": [
{
"path": [
"updateTodo"
],
"data": {
"id": "cac76cab-a71c-48f1-ae23-bd9c529195ad",
"name": "TODO 2",
"description": "My First TODO",
"createdAt": "2022-10-24T19:33:10.203Z",
"updatedAt": "2022-10-24T21:26:51.844Z",
"_version": 9,
"_lastChangedAt": 1666646811867,
"_deleted": null
},
"errorType": "ConflictUnhandled",
"errorInfo": null,
"locations": [
{
"line": 2,
"column": 3,
"sourceName": null
}
],
"message": "Conflict resolver rejects mutation."
}
]
}
So it seems it has to do with the presence of the @belongsTo model in the query.
Instead, the userTodosId
can be used:
Request query with required User
and userTodosId
instead of createdBy { id _deleted }
mutation noNullErrors {
updateTodo(
input: {
description: "No",
id: "cac76cab-a71c-48f1-ae23-bd9c529195ad",
name: "F"
}
) {
id
name
description
createdAt
updatedAt
_version
_lastChangedAt
_deleted
userTodosId """This was added"""
}
}
Response
{
"data": {
"updateTodo": null
},
"errors": [
{
"path": [
"updateTodo"
],
"data": {
"id": "cac76cab-a71c-48f1-ae23-bd9c529195ad",
"name": "TODO 2",
"description": "My First TODO",
"createdAt": "2022-10-24T19:33:10.203Z",
"updatedAt": "2022-10-24T21:26:51.844Z",
"_version": 9,
"_lastChangedAt": 1666646811867,
"_deleted": null,
"userTodosId": "b3f5b495-5962-44cd-b08f-acbfff263133"
},
"errorType": "ConflictUnhandled",
"errorInfo": null,
"locations": [
{
"line": 2,
"column": 3,
"sourceName": null
}
],
"message": "Conflict resolver rejects mutation."
}
]
}
What happens when the User
is not required?
Request query sent when the User
is required and createdBy { id _deleted }
is used
mutation noNullErrors {
updateTodo(input: {description: "No", id: "cac76cab-a71c-48f1-ae23-bd9c529195ad", name: "F"}) {
id
name
description
createdAt
updatedAt
_version
_lastChangedAt
_deleted
createdBy {
id
_deleted
}
}
}
Response
{
"data": {
"updateTodo": null
},
"errors": [
{
"path": [
"updateTodo"
],
"data": {
"id": "cac76cab-a71c-48f1-ae23-bd9c529195ad",
"name": "TODO 2",
"description": "My First TODO",
"createdAt": "2022-10-24T19:33:10.203Z",
"updatedAt": "2022-10-24T21:26:51.844Z",
"_version": 9,
"_lastChangedAt": 1666646811867,
"_deleted": null,
"createdBy": null
},
"errorType": "ConflictUnhandled",
"errorInfo": null,
"locations": [
{
"line": 25,
"column": 3,
"sourceName": null
}
],
"message": "Conflict resolver rejects mutation."
}
]
}
For some reason when the User
is not required the error.data
is not null (I guess it has to do with error handling in AppSync). But notice that the createdBy
field returns a null
value, even though the Todo
has a User
associated (as seen when userTodosId
is used). So the createdBy { id _deleted }
can be safely replaced with the userTodosId
to get that field (I am working in a PR to do that). Later, in your custom conflict handler, you can query the User
by its id
to see if there where any changes to the User
if that is needed.
following. I was getting this, but switched to @hasOne
to work around it for now.
Before opening, please confirm:
JavaScript Framework
React
Amplify APIs
GraphQL API, DataStore
Amplify Categories
api
Environment information
Describe the bug
When a one to many relation is added to the schema that is non-nullable, the DataStore conflict handler does not run. The console prints a number of warnings:
[WARN] 42:54.51 DataStore - conflict trycatch TypeError: Invalid value used in weak set
The network tab also shows multiple attempts at making the mutation with the same ConflictUnhandled response:
This mutation is attempted 11 times. The warning is printed on the console 10 times.
The conflict handler function set in DataStore.configure does not run.
If relation is nullable, the conflict handler does run and those warnings are not printed. There is also only one mutation attempt made and not 11.
It only seems to be affected when there are non-nullable relations and not non-nullable single fields.
Expected behavior
I would expect the conflict handler to always run whether fields are nullable or non-nullable.
Reproduction steps
amplify init
amplify add api
Enable conflict resolution and choose Optimistic Concurrency.
Create a schema with a model with one non-nullable field with @belongsTo directive. Create a corresponding @hasMany field on the target model.
amplify codegen models
amplify push
npm start
Open the app in one window and add some data. Open it in another window and go to dev tools > network (Chrome).
Set throttling to Offline. Make some changes to a record. Make changes to another field on the first browser window. Set throttling to No throttling.
Look for console.log from the conflict handler running. Look for warnings printed in the console.
To make the conflict handler run again, make the @belongsTo field nullable and run:
amplify codegen models
amplify push
Code Snippet
App.js
schema.graphql
Log output
aws-exports.js
Manual configuration
No response
Additional configuration
No response
Mobile Device
No response
Mobile Operating System
No response
Mobile Browser
No response
Mobile Browser Version
No response
Additional information and screenshots
No response