Sequelize findOrCreate() -- .catch() block executed after spread() during no error

Question

My error is similar to this post:

Using sequelize.js as an ORM handler. queryCondition variable value: {where: {$or: [{username:user.username},{email:user.email}]},defaults:user}

Problem: After successfully saving a user instance to the database, the spread() is called as recommended after invoking findOrCreate(). The function block in spread() is invoked, then immediately after the .catch() block is invoked. No clue why this is happening. Can someone please help?

 model.User.findOrCreate(queryCondition).spread(function(user, created){
    callback && callback(user, created);
 }).catch(function(error){
     callback && callback(null, null, error);
 });

{where: {$or: [{username:user.username}, {email:user.email}]},defaults:user} — jried, Sep 06 '16 at 22:02
That sounds like your `callback` did throw an exception. What is the `error`? — Bergi, Sep 06 '16 at 22:17
For cases such as this, you'll want to use the [`then(onFulfilled, onRejected)` pattern](http://stackoverflow.com/a/24663315/1048572) instead of `.then(…).catch(…)`. Or, given Sequelize uses Bluebird, have a look at [`asCallback`](http://bluebirdjs.com/docs/api/ascallback.html) (which does use node callback conventions though, not your custom parameter order). — Bergi, Sep 06 '16 at 22:29
There's no error. Data inserts successfully, but the .catch() block executes immediately after — jried, Sep 06 '16 at 22:32

score 4 · Answer 1 · edited May 23 '17 at 12:22

4

Found the answer: Sequelize uses Bluebird.js as it's promise library (check out the package.json file). The documentation explains the spread() method, which is recommended to use in sequelize 3.0 (see post). A section in this document states that spread() invokes all().then(..) under the covers. Therefore, the following change solved my problem using Node.js:

model.User.findOrCreate(queryCondition) .all().then(function(result, isCreated){ callback && callback(null, result, isCreated); }, function(error) { callback && callback(error); });

Properly resolved promises do not fall into the error block, and actual database errors are caught in the error block. Finally, for those who assumed the error was due to the callback arguments, note that I simply reordered the callback arguments given num8er's suggestion below. :) Hope this helps someone else.

edited May 23 '17 at 12:22

Community

1
1

answered Sep 07 '16 at 07:42

jried

71
1
7

You're calling `callback` with three arguments now, instead of two like you did in your original question. My guess is that _that_ actually solved your problem, and not the rewrite to `.all().then()`. In your initial code, the callback most likely threw an exception (like @Bergi suggested) because it thought that an error had occurred, and that's the reason the `catch` triggered (I assume the callback does `if (err) throw err` or something similar). – robertklep Sep 07 '16 at 08:37
I switched the argument order of my callback() given num8er's suggestion below to match convention...Switching parameters was not the cause of my error. – jried Sep 07 '16 at 17:11

num8er · Answer 2 · 2016-09-07T00:56:59.167

2

If it's invoking .catch block so it seems to have some issue.

Try to debug it and see the reason:

 model.User.findOrCreate(queryCondition).spread(function(user, created){
   callback && callback(user, created);
 }).catch(function(error){
   console.error(queryCondition, error); // add this and check the output
   callback && callback(null, null, error);
 });

also I saw that Your queryCondition variable is also ok.

{
  where:{$or: [{username:user.username},{email:user.email}]},
  defaults:user
}

so let's try this annotation:

{
  where:{$or: [{username: {$eq: user.username}},{email: {$eq: user.email}}]},
  defaults:user
}

read this: http://docs.sequelizejs.com/en/latest/docs/querying/#where

and if with another annotation it will not work properly so I can recommend You to check Your table columns for null, not null fields and compare it with user object to see if there is missing field in user object.

after reading Bergi's comment I'll also recommend You to try to do it like this:

 model.User
   .findOrCreate(queryCondition)
   .then(
     function(result, isCreated){
       callback && callback(result, isCreated);
     },
     function(error) {
       callback && callback(null, null, error);
     });

In case of spread method I think it does not throw exception to be catchable, so in this case:

model.User
  .findOrCreate(queryCondition)
  .spread(function(result, isCreated){
    if(!result) {
      return callback && callback('Cannot create user');
    }
    callback && callback(null, result, isCreated);
  });

P.S. Usually in most of packages it's convention to return error as first argument of callback. So I can only guess that problem happens not in Your code example, it happens somewhere outside that waits for callback.

So try to modify callback like this:

callback(user, created);  =>  callback(null, user, created);

callback(null, null, error);  =>  callback(error);

edited Sep 07 '16 at 00:56

answered Sep 06 '16 at 21:59

num8er

18,604
3
43
57

Thanks for the ordering advice. I'll be sure to refactor...I also added the query condition JSON variable. – jried Sep 06 '16 at 22:07
@jried I've updated my answer, please check Your table columns for constraints about required fields, I can guess that there is some not null fields that are not in Your user object – num8er Sep 06 '16 at 22:10
Tried two things: 1. Adding a 3rd parameter .spread(function(error, user, created) ... Then printed the variable contents: There are only two variables returned from spread. 2. Removed the .catch() statement, this causes an exception if there actually is an error inserting data Essentially, findOrCreate successfully inserts data, and I get the object back ... For some reason .catch() is still called immediately after successful resolve. – jried Sep 06 '16 at 22:32
@jried I did not ment to change spread arguments ))) again can You do console.log(error) inside catch block? – num8er Sep 06 '16 at 22:34
@jried I've updated my answer and added Bergi's proposition, try it. – num8er Sep 06 '16 at 22:40
Hi, .then() does work, but it doesn't catch the 2nd variable (isCreated). Other posts say to use .spread() instead....I also did print the arguments and verified that I get the user object back (json object) but loose the "isCreated" variable. To answer your question about printing the output using spread() and .catch(). The variable queryCondition is correct. It creates the correct SQL statement and inserts correctly...The problem is the .catch() gets called still with "{}" as it's output. – jried Sep 07 '16 at 00:23
Ok. Use spread without catch and only way to check that it did not executed is just check if(!result){} – num8er Sep 07 '16 at 00:47

Sequelize findOrCreate() -- .catch() block executed after spread() during no error

2 Answers2