Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't get the simplest example to work #3

Closed
punkish opened this issue Feb 11, 2019 · 5 comments
Closed

can't get the simplest example to work #3

punkish opened this issue Feb 11, 2019 · 5 comments

Comments

@punkish
Copy link

punkish commented Feb 11, 2019

I am using the following code (combining the Hapi caching tutorial with the catbox-disk documentation) to get started with catbox-disk before I graduate to something more advanced

const Hapi = require('hapi');
const Disk = require('catbox-disk');

const server = new Hapi.Server({
	cache : [{
			name      : 'diskCache',
			engine    : Disk,
			cachePath : '/some/existing/dir',
			cleanEvery: 3600000,
			partition : 'cache'
	}]
});

const start = async () => {

	const add = async (a, b) => {
		await Hoek.wait(1000);   // Simulate some slow I/O
		return Number(a) + Number(b);
	};

	const sumCache = server.cache({
		cache: 'diskCache',
		expiresIn: 10 * 1000,
		generateFunc: async (id) => {
			return await add(id.a, id.b);
		},
		generateTimeout: 2000
	});

	server.route({
		path: '/add/{a}/{b}',
		method: 'GET',
		handler: async function (request, h) {
			const { a, b } = request.params;
			const id = `${a}:${b}`;

			return await sumCache.get({ id, a, b });
		}
	});

	await server.start();

	console.log('Server running at:', server.info.uri);
};

start();

I get the following error

/Users/punkish/Projects/catbox/node_modules/hapi/lib/config.js:19
throw new Error(Invalid ${type} options ${message.length ? '(' + message.join(' ') + ')' : ''} ${result.error.annotate()});
^

Error: Invalid cache options [
{ [1]
"name": "diskCache",
"cachePath": "/Users/punkish/Projects/catbox/diskCache",
"cleanEvery": 3600000,
"partition": "cache",
"engine" [2]: function Disk(options) {\n\n Hoek.assert(\n this.constructor === internals.Connection,\n 'Disk cache client must be instantiated using new'\n );\n\n const settings = Hoek.applyToDefaults(internals.defaults, options);\n\n Hoek.assert(settings.cachePath, 'Missing cachePath value');\n Hoek.assert(\n settings.cleanEvery === parseInt(settings.cleanEvery, 10),\n 'cleanEvery is not an integer'\n );\n\n this.settings = Object.assign({}, internals.defaults, options);\n this.isConnected = false;\n this.cacheCleanerTimeout = null;\n return this;\n}
}
]

[1] "0" must be a Function
[2] "engine" must be an object
at Object.exports.apply (/Users/punkish/Projects/catbox/node_modules/hapi/lib/config.js:19:15)
at module.exports.internals.Core._createCache (/Users/punkish/Projects/catbox/node_modules/hapi/lib/core.js:172:26)
at module.exports.internals.Core._initializeCache (/Users/punkish/Projects/catbox/node_modules/hapi/lib/core.js:140:18)
at new module.exports.internals.Core (/Users/punkish/Projects/catbox/node_modules/hapi/lib/core.js:107:14)
at new module.exports (/Users/punkish/Projects/catbox/node_modules/hapi/lib/server.js:22:18)
at Object. (/Users/punkish/Projects/catbox/index.js:6:16)
at Module._compile (internal/modules/cjs/loader.js:688:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
at Module.load (internal/modules/cjs/loader.js:598:32)
at tryModuleLoad (internal/modules/cjs/loader.js:537:12)

What am I missing?

@punkish
Copy link
Author

punkish commented Feb 11, 2019

one step forward, one step back… in order to get this to work, I have to change the cache declaration code to below

cache: [{
	name: 'diskCache',
	engine: new Disk({
	cachePath: '/Users/punkish/Projects/catbox/diskCache',
	cleanEvery: 3600000,
	partition: 'cache'
       })
}]

I also have to change return await sumCache({ id, a, b }); to return await sumCache.get({ id, a, b }); else I get the error that sumCache is not a function

So, the above works when I go to, say, /add/5/2. However, if I change the params, say, /add/5/9 I get a different error Cannot provision the same cache segment more than once

This is very frustrating, and the mismatch between the documentation and what should be seems to be really uncharacteristic. Suggestions?

@EyePulp
Copy link
Collaborator

EyePulp commented Feb 11, 2019

Hey @punkish
Sorry for the frustration, the readme could definitely use an update for v17 and above.

I got your code working locally with the following changes (some you might have left out of your example for clarity):

  • require Hoek
  • added a fixed port value for easier testing
  • added a segment name in the server.cache options (this is the only substantive change I made)

After doing this, I was successfully calling multiple /add/x/y routes and getting fast secondary calls to repeated routes. I'm running:

  • Hapi v18.1.0
  • Catbox v10.0.6
  • Node v10.15.1
  • catbox-disk v3.0.0
const Hoek = require('Hoek');
const Hapi = require('hapi');
const Disk = require('catbox-disk');

const server = new Hapi.Server({
    port: 9999,
    cache: [{
        name: 'diskCache',
        engine: new Disk({
            cachePath: './',
            cleanEvery: 3600000,
            partition: 'cache'
        }),
    }]
});

const start = async () => {

    const add = async (a, b) => {
        await Hoek.wait(1000);   // Simulate some slow I/O
        return Number(a) + Number(b);
    };

    const sumCache = server.cache({
        cache: 'diskCache',
        segment: 'foo',
        expiresIn: 10 * 1000,
        generateFunc: async (id) => {
            return await add(id.a, id.b);
        },
        generateTimeout: 2000
    });

    server.route({
        path: '/add/{a}/{b}',
        method: 'GET',
        handler: async function (request, h) {
            const { a, b } = request.params;
            const id = `${a}:${b}`;

            return await sumCache.get({ id, a, b });
        }
    });

    await server.start();

    console.log('Server running at:', server.info.uri);
};

start();

I'll get an updated readme out there momentarily. Let me know if the above example fails for you.

@punkish
Copy link
Author

punkish commented Feb 11, 2019

Many thanks @EyePulp. Yes, your suggested code works. The key was to use engine: new Disk(options) as I noted in my second comment above, which I figured out by looking at your code. However, I think the subsequent problem that I reported, that of the error Cannot provision the same cache segment more than once, that is likely because of my lack of understanding of how things work in the Hapi world. See, I moved the route definition to its own file. So now I have add.js that looks like so

const Hoek = require('hoek');
const add = async (a, b) => {
    await Hoek.wait(1000);   // Simulate some slow I/O
    return Number(a) + Number(b);
};

module.exports = {
    path: '/add/{a}/{b}',
    method: 'GET',
    handler: async function (request, h) {

        const sumCache = request.server.cache({
            cache: 'diskCache',
            expiresIn: 10 * 1000,
            segment: 'foo',
            generateFunc: async (id) => {
                return await add(id.a, id.b);
            },
            generateTimeout: 2000
        });

        const { a, b } = request.params;
        const id = `${a}:${b}`;

        return await sumCache.get({ id, a, b });
    }
}

and the index.js edited to as below

const start = async () => {

    server.route([
        require('./routes/add')
    ]);

    await server.start();

    console.log('Server running at:', server.info.uri);
};

and that is why I getting the error. A new request causes a new invocation of sumCache() leading to the error. My question is thus: how can I access the server variable in an external route file?

@punkish
Copy link
Author

punkish commented Feb 11, 2019

ok, if I add module.exports = server to my index.js and then, in add.js I have const server = require('../index') then it all works. Of course, I also have to define sumCache without the request. My add.js file now looks like this

const Hoek = require('hoek');
const server = require('../index')

const add = async (a, b) => {

    await Hoek.wait(1000);   // Simulate some slow I/O
    return Number(a) + Number(b);
};

const sumCache = server.cache({
    cache: 'diskCache',
    expiresIn: 10 * 1000,
    segment: 'foo',
    generateFunc: async (id) => {
        return await add(id.a, id.b);
    },
    generateTimeout: 2000
});

module.exports = {
    path: '/add/{a}/{b}',
    method: 'GET',
    handler: async function (request, h) {

        const { a, b } = request.params;
        const id = `${a}:${b}`;

        return await sumCache.get({ id, a, b });
    }
}

My question, is that the right way to do things? That is, if I want to keep my routes in separate files but still access the server methods?

@EyePulp
Copy link
Collaborator

EyePulp commented Feb 11, 2019

I'm going to close this issue for now, as it seems like we're past the catbox-disk specific issues. The rest is opinion and you can ignore it all you like =P

The "hapi" way to break up routes into separate files is generally to create them as a plugin, which gives you control over how they are initialized (and you can bind values/functions to all routes as part of their this context, which is super helpful). Full explanation here: https://hapijs.com/tutorials/plugins

I tend to group related routes into their own plugins so they can share overlapping functionality, and I typically put each plugin in its own folder for keeping things clean. For this example I broke things into your index.js (the main server) and summer.js in the same folder (since all its logic fits cleanly in a single file). Your index.js gets a lot cleaner when you start to break out plugins, and plugins are easy to unit test or turn into node modules to re-use between hapi projects.

I wrote the following from memory, and haven't run it - some debugging may be required:

// index.js
const Hapi = require('hapi');
const Disk = require('catbox-disk');

const server = new Hapi.Server({
    port: 9999,
    cache: [{
        name: 'diskCache',
        engine: new Disk({
            cachePath: './',
            cleanEvery: 3600000,
            partition: 'cache'
        }),
    }]
});

const start = async () => {

    await server.register({ plugin: require('./summer') });
    await server.start();

    console.log('Server running at:', server.info.uri);
};

start();
// summer.js
const Hoek = require('Hoek');

module.exports = {
    plugin: {
        name: 'summer-plugin',
        register: async function(server, options) {

            const sumCache = server.cache({
                cache: 'diskCache',
                segment: 'foo',
                expiresIn: 10 * 1000,
                generateFunc: async (id) => {
                    return await add(id.a, id.b);
                },
                generateTimeout: 2000
            });

            server.bind({ sumCache }); // binds sumCache to every route registered **within this plugin** after this line

            server.route([
                { path: '/add/{a}/{b}', method: 'GET', handler },
            ]);
        },
    },
};

const handler = async function(request, h) {
    const { a, b } = request.params;
    const id = `${a}:${b}`;
    return await this.sumCache.get({ id, a, b }); // uses the bound sumCache instance from index.js
};

const add = async (a, b) => {
    await Hoek.wait(1000);   // Simulate some slow I/O
    return Number(a) + Number(b);
};

@EyePulp EyePulp closed this as completed Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants