TypeError: Cannot read property 'indexes' of undefined

[felixlberg]

Hello I think I encountered the following error.

To Reproduce

Step by step instructions to reproduce the behavior:

clone apostrophe-boilerplate, add apostrophe-caches-redis. You need to have a empty or not existing database and migration fails at: apostrophe-caches.removeBadIndex

Completed database migration: apostrophe-filesSlugPrefix
Running database migration: apostrophe-caches.removeBadIndex
TypeError: Cannot read property 'indexes' of undefined
    at Object.callback (/home/user/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-caches/index.js:253:54)
...

Details

Version of Node.js: v12.22.1

Version of npm: 7.17.0

Server Operating System: Ubuntu 20.04.5

Version of Redis Redis server v=5.0.7

Version of mongodb 3.6.8

Version of apostrophe 2.224.0

[BoDonkey]

Hi @felixlberg, I'm actually not able to replicate this error (I think). Having said that, we actually don't support node 12. It has reached its end-of-life date. I did get errors with the install of apostrophe-redis-caches. Please try again with Node 14 or 16 (we are currently testing 18) to see if this error repeats. Thanks! Bob

[felixlberg]

Hi @BoDonkey I can confirm that this error happens with Node v16.19.0. I've reproduced the error now on 2 separate machines. On the other machine I'm using MongoDB 6.0.4 and Ubuntu 18.04 and Redis server v=4.0.9.

Steps to reproduce:

git clone https://github.com/apostrophecms/apostrophe-boilerplate.git
cd apostrophe-boilerplate
npm install --save apostrophe-caches-redis
configure app.js
npm start

It is important that you drop the database if you done a migration before so that apostrophe runs the database migration.

you get the following error:

Running database migration: apostrophe-caches.removeBadIndex
TypeError: Cannot read properties of undefined (reading 'indexes')
    at Object.callback (/home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-caches/index.js:253:54)
    at /home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-migrations/lib/implementation.js:197:30
    at tryCatcher (/home/felix/github/apostrophe-boilerplate/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/home/felix/github/apostrophe-boilerplate/node_modules/bluebird/js/release/method.js:39:29)
    at fn (/home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-migrations/lib/implementation.js:196:29)
    at /home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-migrations/lib/implementation.js:203:14
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/utils.js:704:5
    at executeCallback (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/operations/execute_operation.js:65:7)
    at handleCallback (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/utils.js:109:55)
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/operations/find_one.js:31:9
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/utils.js:704:5
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/cursor.js:256:9
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:746:9
    at handleCallback (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:32:5)
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:690:38
    at Cursor._endSession (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:404:7)
Unhandled rejection TypeError: Cannot read properties of undefined (reading 'indexes')
    at Object.callback (/home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-caches/index.js:253:54)
    at /home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-migrations/lib/implementation.js:197:30
    at tryCatcher (/home/felix/github/apostrophe-boilerplate/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/home/felix/github/apostrophe-boilerplate/node_modules/bluebird/js/release/method.js:39:29)
    at fn (/home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-migrations/lib/implementation.js:196:29)
    at /home/felix/github/apostrophe-boilerplate/node_modules/apostrophe/lib/modules/apostrophe-migrations/lib/implementation.js:203:14
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/utils.js:704:5
    at executeCallback (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/operations/execute_operation.js:65:7)
    at handleCallback (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/utils.js:109:55)
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/operations/find_one.js:31:9
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/utils.js:704:5
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/cursor.js:256:9
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:746:9
    at handleCallback (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:32:5)
    at /home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:690:38
    at Cursor._endSession (/home/felix/github/apostrophe-boilerplate/node_modules/mongodb/lib/core/cursor.js:404:7)

Btw I saw in the commits that you updated redis to next major version last week:

  "redis": "^2.8.0"
  "redis": "^3.1.1"

Maybe some breaking changes which affect the removeBadIndexMigration function now?

[BoDonkey]

I'm thinking that this is a MongoDB 6 problem, but you may be right about the redis update. I'll try to check this out on a container later today.

[BoDonkey]

Thanks for bringing this to our attention @felixlberg. We have opened a work ticket. In the meantime I think you should be able to spin the site up once (or even add an admin user) to trigger the migrations prior to installation of the apostrophe-caches-redis package. Not sure if this is possible in your case.

[felixlberg]

Hey @BoDonkey I think he issue is with the implementation of the ensureIndexes method. The original implementation assumes that the underlying data store is a MongoDB collection, but the comments indicate that the code is being used with Redis, which does not use collections.

The method is later overridden, but the override only returns a callback with a null value, which does not properly handle the subsequent use of the indexes property on the cacheCollection.

    // Override the ensureIndexes method, which is not needed since we are
    // not using mongo collections.
    self.ensureIndexes = function(callback) {
      return callback(null);
    };

I resolved the issue with overriding the the removeBadIndexMigration cause as said with redis we are not using collections. So I added the following to index.js of apostrophe-caches-redis

    // Override removeBadIndexMigration function, which is not needed since we are
    // not using mongo collections
    self.removeBadIndexMigration = function() {
      self.on('apostrophe:modulesReady', 'addRemoveBadIndexMigration', function() {
        try {
          self.apos.migrations.add(self.__meta.name + '.removeBadIndex', async function() {
            // remove the code that references the "indexes" property
          });
        } catch (error) {
          console.error(error);
        }
      });
    };

The migration works now without issues but I don't know the implications of the removeBadIndexMigration function regarding redis and if apostrophe-caches-redis isn't using collections in general or if it just don't use collections for the ensureIndexes method?

Anyway when apostrophe-caches-redis isn't using mongo collections in general it would be save to override the removeBadIndexMigration I think.

Alternatively, you can modify the ensureIndexes method to properly handle the creation of indexes in Redis but i don't know enough about redis to answer this question.

[felixlberg]

additionally I found out that this part will get deprecated soon:

    self.on('apostrophe:destroy', 'closeRedisConnection', function() {
      self.client.stream.removeAllListeners();
      self.client.stream.destroy();
    });

Cause the removeAllListeners and destroy methods were used in earlier versions of the Rredis to close a connection. However, in recent versions of the redis, the recommended method for closing a connection is to call the quit method on the Redis client instance. So I have rewritten this function too:

    self.on('apostrophe:destroy', 'closeRedisConnection', function() {
      self.client.quit();
    });

[boutell]

Felix, I like your solution of overriding the migration, I think that is appropriate and I would accept a PR for that.

On Fri, Feb 10, 2023 at 10:10 AM Felix @.***> wrote:

additionally I found out that this part will get deprecated soon too:

self.on('apostrophe:destroy', 'closeRedisConnection', function() {
  self.client.stream.removeAllListeners();
  self.client.stream.destroy();
});

Cause the removeAllListeners and destroy methods were used in earlier versions of the Rredis to close a connection. However, in recent versions of the redis, the recommended method for closing a connection is to call the quit method on the Redis client instance. So I have rewritten this function too:

self.on('apostrophe:destroy', 'closeRedisConnection', function() {
  self.client.quit();
});

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe-caches-redis/issues/12#issuecomment-1425943086, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27LNWSGVCWUHZHGHPLTWWZK5BANCNFSM6AAAAAAUXETZ3I . You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

[felixlberg]

@boutell Done...

[BoDonkey]

Closed by PR #13.