Closed arkadiybutermanov closed 1 month ago
I am able to reproduce the bug.
The command is correctly built and the flag is correctly set. However, when processing the response, electrodb
ends up discarding the Item
property.
I was able to trace the bug down to the ownsItem(item)
function in /electrodb/src/entity.js
.
this.getName() === item[this.identifiers.entity] &&
During a transaction, this line resolves to false, because item[this.identifiers.entity]
returns an invalid value.
In my example, it should be returning license
, but it returns { S: 'license' }
.
Some extra information:
ownsItem(item) {
console.log('ownsItem', item, this.getName(), item[this.identifiers.entity] )
return (
item &&
this.getName() === item[this.identifiers.entity] &&
this.getVersion() === item[this.identifiers.version] &&
validations.isStringHasLength(item[this.identifiers.entity]) &&
validations.isStringHasLength(item[this.identifiers.version])
);
}
logs
ownsItem {
indexedName: { S: 'professional' },
deleteAt: { N: '1753710692' },
__edb_e__: { S: 'license' },
licenseTypeUid: { S: 'professional' },
GSI2PK: { S: '3#user-1' },
orgId: { N: '3' },
entityVersion: { N: '1' },
createdAt: { S: '2023-11-22T16:24:55.776Z' },
GSI1PK: { S: 'licenses_by_username#3' },
GSI2SK: { S: 'professional' },
GSI1SK: { S: 'professional#professional' },
name: { S: 'Professional' },
SK: { S: 'user-1#professional' },
__edb_v__: { S: '1' },
PK: { S: 'licenses#3' },
userUid: { S: 'user-1' }
} license { S: 'license' }
This is how I run it:
App.transaction
.write(({ license }) => [
license
.update({
licenseTypeUid: 'professional',
orgId: 3,
userUid: 'user-1',
})
.set({
name: 'Professional',
createdAt: '2023-11-22T16:24:55.776Z',
})
.where(notDeletedCondition)
.commit({
params: {
ReturnValuesOnConditionCheckFailure: 'ALL_OLD',
},
}),
])
.go()
Interesting, so the response back is unmarshalled? What version of the sdk is being used to recreate this?
The response from Electrodb lacks the Item property. We get the exception back with the Code: ConditionCheckFailure
but it lacks the Item
property.
We are currently using aws-sdk 3.552.0 (its the NODEJS_20_X Lambda runtime). We discussed it with an AWS SDK maintainer and came to the conclusion the bug is not on aws-sdk.
Here is the discussion: https://github.com/aws/aws-sdk-js-v3/issues/6237
Thanks in advance,
Thank you for all the detail here, I will look into this
hi @tywalch , where you able to research this issue?
Thanks,
@dil-mvallone I have and I have some things cooking locally on my end, though not at the pace I'd normally like. I don't think this is a heavy lift outside of my recent availability. Thank you for actively following up, I appreciate the ping!
hi @tywalch, any progress on this issue? If we were to research and create a PR would you review it and merge it afterwards? Thanks,
I have not made progress. I did start to look at this, but I quickly realized that it doesn't fit nicely into electro's existing return interface. Could you comment on the expected return interface you'd expect for this type of request? Presumably you'd expect some sort of feedback that request failed but you'd also prefer for the request to not reject and the values to be available similar to how they currently are returned.
hi @tywalch. I began preparing a PoC to better explain the issue, but I could not reproduce the bug on the PoC. Then I went back to our original issue on our codebase and the bug was not reproducible anymore. I did more research and it seems that version 2.14.2 fixed this issue. I was consistently able to reproduce it on 2.14.1.
We can close this issue for now and I'll create a new one if I find this bug again.
Thanks!
Describe the bug Since Sep 2023, all write operations support
ReturnValuesOnConditionCheckFailure
property, however there is no way to set that flag using ElectroDB currently except for transactional operations: https://github.com/tywalch/electrodb/blob/773e0ba6a1d9c557608e7b731eeef80be9502f83/src/entity.js#L1733It's quite important to us as it allows identifying cause of failure without making separate request.
ElectroDB Version 2.14.1
Expected behavior An additional option translating to
ReturnValuesOnConditionCheckFailure
(e.g.failureResponse
) must be available for all write operations along with the existingresponse
(ReturnValues
) option.