Client insights

[andreafspeziale]

Hello Josh!

I wanted to reach out and first express my gratitude for your Memcached client implementation. After extensively testing various Memcached clients in the JavaScript ecosystem, I've found yours to be the most compelling solution for several reasons:

• It's one of the most recently maintained implementations
• It offers a clean and straightforward approach
• It's minimal yet effective, unlike other clients that include poorly implemented features
• It's compatible with Bun
• It provides TypeScript support

I previously attempted to contact you through other channels without success. While I was hesitant to open an issue just to start a conversation, seeing your recent active contributions encouraged me to reach out this way.

I'm currently developing a Memcached wrapper for the NestJS backend framework. As I'm working on version 4, I'd like to switch to your client implementation (Pool). However, I've encountered some questions and concerns that I'd really appreciate your insights on:

1. I've implemented SuperJSON for value serialization, as it allows caching of boolean and object types beyond the current string and number limitations. I'd appreciate your thoughts on this.
2. Regarding error handling, I've noticed that .set() and .get() operations return boolean values without throwing exceptions. While this helps identify cache misses and successful operations, it makes it challenging to distinguish connection issues from other failures. The only solution I've found is running .ping() before operations, though this doesn't guarantee connection stability during execution. Do you have any recommendations for more robust error handling?
3. One notable feature I've seen in other implementations is data compression. As this isn't my area of expertise, I'd value your perspective on potentially incorporating compression capabilities into the client.

I would be incredibly grateful for any opportunity to discuss these points with you. If you'd prefer to continue this conversation through other channels, you can find all my contact information on my public GitHub profile. Your insights would be invaluable in helping shape the development of my wrapper.

Looking forward to hearing your thoughts!

[joshbetz]

I've implemented SuperJSON for value serialization, as it allows caching of boolean and object types beyond the current string and number limitations. I'd appreciate your thoughts on this.

Yep, makes sense. I've done similar in some of my projects.

Do you have any recommendations for more robust error handling?

Good question. I have mainly used the HashPool where I've implemented a polling check https://github.com/joshbetz/node-memcached/blob/main/src/hashpool.ts#L57-L61

One notable feature I've seen in other implementations is data compression. As this isn't my area of expertise, I'd value your perspective on potentially incorporating compression capabilities into the client.

This is an interesting idea. Do you have examples?

[andreafspeziale]

Yep, makes sense. I've done similar in some of my projects.

Great, thx for the feedback!

Good question. I have mainly used the HashPool where I've implemented a polling check

Understood. But even leveraging the polling check feature (which I recommend to add in the README ❤️) I believe consumers will still be in trouble.

import { Pool } from '@joshbetz/memcached' ;

const memcached = new Pool( 11211, 'localhost' );

const key = 'key';
const cached = await memcached.get(key);

if (cached) { // <-- If cached === false we don't really know if it is a cache miss or a connection issue
...
}

await memcached.set(key, 'value'); // <-- So at this point, does it really makes sense to "set"? Maybe there was a connection issue

Even in the following scenario:

import { Pool } from '@joshbetz/memcached' ;

const memcached = new Pool( 11211, 'localhost' );

const key = 'key';
let cached: string;

// Between A and B we can still have connection issues
const ready = await memcached.ready(); // <-- A

if (ready) {
  cached = await memcached.get(key); // <-- B
} else {
  throw new Error('Connection issue')
}

if (cached) { // At this point we still don't know if there was a cache miss or a connection issue
...
}

await memcached.set(key, 'value');

Please correct me if I'm missing something.

Would it be an option to return null instead of a boolean on cache misses? Or any similar strategy to be able to correctly react to the different situations.

This is an interesting idea. Do you have examples?

Sure! Node.js provides the node:zlib module which can be used to compress data.

memcached-plus is using it on an API request basis.

This is the util.

Side note: I believe .npmignore needs to be update. E.g CI and more is in the package ☺️

[joshbetz]

Would it be an option to return null instead of a boolean on cache misses? Or any similar strategy to be able to correctly react to the different situations.

Can you tell me more about the use case? What would you do differently if you knew it was a connection failure at the time of the query as opposed to polling for the connection status?

Sure! Node.js provides the node:zlib module which can be used to compress data.

IMO this is outside the scope of the library for now.

[andreafspeziale]

Hi Josh! And thank you in advance for your consideration.

Can you tell me more about the use case? What would you do differently if you knew it was a connection failure at the time of the query as opposed to polling for the connection status?

Let's assume that interacting with Memcached is part of an HTTP request/response flow.

If data cached:

• return cached data

otherwise

• retrieve fresh data
• set data into cache
• return data

I believe during traffic spikes cache, even if super fast, will be under pressure.
It means:

• .ping Memcached would add an extra step and pressure point
• the chance that connection issues happen between .ping and actually trying to .get data increase
• if I'm not sure that .get is returning false cause I have an actual cache miss I may add extra (overall) pressure retrieving fresh data and trying to .set it into cache

More generally, I think that if it is not possible to interact with Memcached for any reason, the code should break or raise some kind of exception.

Let me know your thoughts. It is also possible that I'm missing something from your client and what I'm describing is already possible tweaking it. I'm also interested to eventually listen how would you leverage pingInterval or retries in the context I described. I can definitely see pingInterval very useful for healthz checks but not really useful in procedural code contexts.

IMO this is outside the scope of the library for now.
Fair enough!

Side note: in HashPool and other constructors you are using any, (..., opts?: any). Wouldn't be better using the actual types?

[joshbetz]

if I'm not sure that .get is returning false cause I have an actual cache miss I may add extra (overall) pressure retrieving fresh data and trying to .set it into cache

So you want to avoid the .set if Memcached is down?

[andreafspeziale]

I would like to understand if .get is returning false because of cache miss or some other issue without applying any fallback call like .ping. It could be achieved by returning something like false or null in case of cache miss and throwing an error in case of connection issue.*

I would do the same also in case of .set. Set commands can fail because of connection issue, caching key format and caching value size. I believe throwing an error would be better than just returning a boolean.

E.g if connection is ok and you try to cache something really big without knowing caching value limits, returning just false is a lil bit weak for the consumer.

* from your latest commit I learned it was possible (only) on Pool client by using forwardPoolErrors:

import { Pool } from "@joshbetz/memcached";

const memcached = new Pool(11211, "localhost", { forwardPoolErrors: true });

const cached = await memcached.get("key");

if (!cached) {
  console.log("Not cached");
  const setResult = await memcached.set("key", "my-value");
  console.log("Set result: ", setResult);
} else {
  console.log("Cached: ", cached);
}

process.exit(0);

In version 1.3.1, Pool client using { forwardPoolErrors: true } shows some kind of error which could be used to throw...

ECONNREFUSED: Failed to connect
   errno: 53
 syscall: "connect"

Why did you removed it? I think throw expressive exceptions is the way to go.

[joshbetz]

You can still listen for the error event https://github.com/joshbetz/node-memcached/blob/main/src/pool.ts#L48

[andreafspeziale]

Thank you, that's great!
In a class-oriented scenario where the client is injected, how would you suggest handling errors effectively?

To keep it simple, I might implement something like this:

import { Pool } from "@joshbetz/memcached";

class MyMemcachedWrapper {
  constructor(private readonly client: Pool) {
    this.client.on("error", (error) => this.throwOnError(error));
  }

  throwOnError(error: Error) {
    throw new Error(`The following error occurred: ${error.message}`);
  }

  myWrappedGet(key: string) {
    return this.client.get(key);
  }
}

try {
  const client = new Pool(11211, "localhost");
  const myMemcachedWrapper = new MyMemcachedWrapper(client);

  const cached = await myMemcachedWrapper.myWrappedGet("myKey");

  console.log(cached);
  process.exit(0);
} catch (e) {
  console.error(e);
  process.exit(1);
}