clarkie / dynogels

DynamoDB data mapper for node.js. Originally forked from https://github.com/ryanfitz/vogels
Other
490 stars 110 forks source link

.destroy(x) does not throw error if [x] does not exist #44

Closed csi-lk closed 7 years ago

csi-lk commented 7 years ago

As per title, don't know if intended but was expecting an error if deleting on a hash key that does not exist, instead it silently fails.

Example of bug:

const Account = dynogels.define('account-email', {
  hashKey: 'name',
  schema: {
    email: Joi.string().email(),
  },
})

Account.create({
  email: 'foo@example.com',
})

Account.destroy({
  email: 'bar@example.com',
}, (err, data) => {
  console.log(err, data)
  // returns null, null
})

Workaround would be to ReturnValues: 'ALL_OLD' and see if data existed to manually throw error

High level Example:

Account.destroy({
  email: 'bar@example.com',
}, {
  ReturnValues: 'ALL_OLD',
}, (err, data) => {
  if(!data){
    throw 'No email exists'
  } else {
    return 'email deleted'
  }
})

Cheers

jkav77 commented 7 years ago

This can be solved with a conditional check similar to #39. I will look at adding a conditional check to the destroy method that will fail if the item does not exist.

jkav77 commented 7 years ago

The solution is below. See #53

const Account = dynogels.define('account-email', {
  hashKey: 'id',
  schema: {
    id: Joi.number(),
    email: Joi.string().email(),
  },
})

Account.destroy(
  { id: '123' },
  { expected: { id: { Exists: true } } },
  (err, data) => {
    console.log(err) // Conditional Check failed
});
cdhowie commented 7 years ago

I would say this is expected behavior (without the expected clause). I can't think of any database that will raise an error if you attempt to delete something that does not exist. Even in a SQL database -- a DELETE where no records match the WHERE clause is just a successful no-op.

clarkie commented 7 years ago

I'm with @cdhowie this is expected behaviour.