Type of error returned by stores about seneca-store-test HOT 17 CLOSED

yes - very good suggestion - let's create a standard set of errors!

from seneca-store-test.

Let's brainstorm some of the possible errors.
I suggest

• duplicate primary key
• connection related
• timeout
• generic

from seneca-store-test.

That list looks good:

Perhaps also:

• not found

from seneca-store-test.

not found is NOT an error :)
it's a fact about the data - be very careful with this one
you just get a null result, or an empty array

errors in Seneca relate to failure of the underlying system - db connection down etc, or a db error like a foreign key fail, duplicate etc

@paolochiodi please use the https://github.com/rjrodger/eraro package to manage the codes and msgs

from seneca-store-test.

@rjrodger Good point and an important one for the docs!

@paolochiodi We can add these to the docs when you have codified them. So no not found just null or empty array. I think your other errors pretty much cover everything else.

from seneca-store-test.

@rjrodger @mcdonnelldean's error suggestion led me to another question what happen if?

1. I try to remove a non-existing record
2. I try to update a non-existing record

My take is that we should just silently do nothing, but would not be clear what we should pass as result (especially for point 2)

from seneca-store-test.

I'll have to defer to @rjrodger on this one

1. I've seen examples of silent and explicit failure, personally I'm a fan of silent for remove.
2. There was some talk about upserting that this might relate to.

I'd say these ones are for @rjrodger until codified.

from seneca-store-test.

on 1. we go with idempotency - it always works - as the final state of the db is the same, it can never be an error
=> load$ may not return data - need to document this

from seneca-store-test.

1. inserts - but i think this happens anyway? .save$ should update if exists when given id$
   probably requires some business logic on the db that have an explicit INSERT vs UPDATE

from seneca-store-test.

1. save works in 3 different ways:
   a. without id: auto-generetates an id and inserts
   b. with id$: always tries to insert a new record with the given id (this may lead to duplicate key error)
   c. with id: update the record

My last question was about point c: what happens if there's no record with the given id? Is it ok to to insert or should only update a record if it already exists (like every sql database does)?

Imho, this is a tricky decision with no correct answer because it's difficult to understand what is the desired outcome.
I.e. when trying to update, but another process has already deleted the record.

Since I mostly use sql databases I'm biased toward not doing anything

from seneca-store-test.

...what happen if?

1. I try to remove a non-existing record
2. I try to update a non-existing record

I suggest no error and the returned data indicates that 0 records were successfully modified. i.e.

remove/update(query, data, function(err, result) {
  if(!err && result && !result.numRows) {
    // Possibly because non-existing record
  }
});

from seneca-store-test.

The whole generates-id thing is actually a pain because it means you can't just let sql sequences handle the incrementing the id for you.

from seneca-store-test.

@AdrianRossouw on the specific issue I agree with you. That's why added a store-specific option to the seneca-mysql-store to make him use the auto-increment primary keys of the db instead of generating one in code.
I think store specific option is the best way to go for seneca.

On a more general point of view, I think uuid are superior to auto-incrementing integer ids for two reasons:

• easier to coordinate in shards / replicas
• more secure, especially if id is exposed to consumers (i.e. through api): auto increment ids expose information about the db (record age, total number of records) and make easier for an attacker to guess the id of a record

from seneca-store-test.

@paolochiodi relational db stores have a subset of additional sementics, such as incrementing ids, and this update/insert question, so it's perfectly fine for them to have additional "standard" behavior.

And there may well be an argument for having a relational store "super class plugin" that implements this consistently, that the other relation plugins extend - same idea as the transports.

@guyellis the result is an entity object - a null result implicitly indicates zero rows

@AdrianRossouw and in particular, all relational stores should support auto-increment (and in the general case, db system provided) ids. I would however concur with @paolochiodi on the general advisability of auto-increments - really not scalable at all at all :)

On the insert/update question - seneca stores provides a slightly extended key-value data model - that's pretty much it. So the behaviour in this case is the question - and if we prefer availability over consistency (we do :) ), then the "correct" behaviour is to INSERT if the UPDATE fails.

Seneca does not make guarantees wrt data integrity - and this is deliberate. Many microservices use cases are for local data anyway.

We really have to resist the urge to create a full featured ORM here. Instead we define only a simple, scalable, data model (without relations), and advise use of a real ORM (mongoose etc) if traditional database functionality is desired

/cc @geek @colmharte @mcdonnelldean

from seneca-store-test.

@rjrodger I agree that relational stores may have additional standard behaviour as long those are actually "additional" and not "different". Put another way, I think relational store standards may require additional features, not have features work differently.

On the insert/update/upsert things, I have another observation. This is around case 1.b in my previous comment.
Do we still need the "id$" option on save? Won't just and "id" obtain the same result?
The only difference right now is that "save with id$" will fail if id already exists, while "save with id" will update the record instead.

Do we want to keep this difference?

from seneca-store-test.

Back to the topic of the issue, @rjrodger do you have any specific indication for the error objects?
Do you want them to be instances of specific error objects that will be included in seneca? Is an object with an error code and/or message enough?

from seneca-store-test.

@rjrodger discard previous comment - I re-read the issue an noticed that you mentioned eraro

from seneca-store-test.