kuzzleio / kuzzle

Open-source Back-end, self-hostable & ready to use - Real-time, storage, advanced search - Web, Apps, Mobile, IoT -
https://kuzzle.io
Apache License 2.0
1.44k stars 124 forks source link

Error when removing room for customer leaves the room marked as destroyed #740

Closed benoitvidis closed 7 years ago

benoitvidis commented 7 years ago

Following a client disconnection:

kuzzle_1         | 0|KuzzleSe | Unhandled rejection TypeError: Cannot read property 'collection' of undefined
kuzzle_1         | 0|KuzzleSe |     at removeFromTestTables (/var/app/lib/api/dsl/storage/index.js:427:29)
kuzzle_1         | 0|KuzzleSe |     at Storage.remove (/var/app/lib/api/dsl/storage/index.js:210:3)
kuzzle_1         | 0|KuzzleSe |     at RealtimeEngine.remove (/var/app/lib/api/dsl/index.js:96:23)
kuzzle_1         | 0|KuzzleSe |     at HotelClerk.cleanUpRooms (/var/app/lib/api/core/hotelClerk.js:468:28)
kuzzle_1         | 0|KuzzleSe |     at HotelClerk.removeRoomForCustomer (/var/app/lib/api/core/hotelClerk.js:534:23)
kuzzle_1         | 0|KuzzleSe |     at async.each (/var/app/lib/api/core/hotelClerk.js:156:31)
kuzzle_1         | 0|KuzzleSe |     at /var/app/node_modules/async/dist/async.js:3025:16
kuzzle_1         | 0|KuzzleSe |     at eachOfArrayLike (/var/app/node_modules/async/dist/async.js:941:9)
kuzzle_1         | 0|KuzzleSe |     at eachOf (/var/app/node_modules/async/dist/async.js:991:5)
kuzzle_1         | 0|KuzzleSe |     at Object.eachLimit (/var/app/node_modules/async/dist/async.js:3089:3)
kuzzle_1         | 0|KuzzleSe |     at Promise (/var/app/lib/api/core/hotelClerk.js:155:13)
kuzzle_1         | 0|KuzzleSe |     at HotelClerk.hotelClerkRemoveCustormerFromAllRooms [as removeCustomerFromAllRooms] (/var/app/lib/api/core/hotelClerk.js:154:12)

At a first glance, besides the error handling problem, it looks like a wrong argument is passed from the HotelClerk to the DSL, which expects a filter id.

scottinet commented 7 years ago

Nice catch!

Found a way to reproduce it 100% of the time. Here are the steps:

Sample code with the JS SDK:

var Kuzzle = require('kuzzle-sdk');

var kuzzle = new Kuzzle('localhost', function (err, res) {
  if (err) {
    console.error(err);
    process.exit(1);
  }

  kuzzle.collection('foo', 'bar').subscribe({or: [{equals: {foo: 'bar'}}, {exists: {field: 'foo'}}]}, () => {})
  .onDone((err, room1) => {
    kuzzle.collection('foo', 'bar').subscribe({equals: {foo: 'bar'}}, () => {})
      .onDone((err, room2) => {
        room1.unsubscribe();
        setTimeout(() => {console.log('...done'); process.exit(0); }, 10000);
      });
  });
});

The bug lies in the DSL storage.store() method: when a filter is registered, containing only already existing subfilters, it does not increment the filters count in the testTables structure (the conditions index). When then removing a filter, we decrement the filters count. If it reaches 0, that triggers an index removal. If that index is the last one on a given collection, the structure is cleaned up. After that, if another unsubscribe occurs, it tries to clean up the testTables structure... which is now empty.

benoitvidis commented 7 years ago

The problem occurs in quite a specific case: when a new filter is created which contains only existing conditions. In this situation, the test table filter count is not incremented and the filter fidx index (contains the index of correlated test table entry) is not updated.