Unable to cache a value of null

[Telokis]

Prerequisites

Please answer the following questions for yourself before submitting an issue.

• [x] I am running the latest version
• [x] I checked the documentation and found no answer
• [x] I checked to make sure that this issue has not already been filed
• [x] I'm reporting the issue to the correct repository

Current Behavior

null and undefined cannot be cached by moleculer.

Expected Behavior

I expected at least null to be a cacheable value.

Failure Information

More often than not, a specific action will be invalid. For example, trying to get a specific id from database but it doesn't exist.
In such a case, it makes sense to return null but I don't want to be flooded by requests since I know it won't return anything other than null until I ask the cache to be cleaned.

Steps to Reproduce

1. Create a service containing an action wich returns null.
2. Enable caching for the action.
3. Add a sleep in order to voluntarily slow the action.
4. Try to call the action, notice it takes time.
5. Try to call the action again, the caching didn't occur and it still takes time.

Reproduce code snippet

const broker = new ServiceBroker({
    logger: console,
    transporter: "NATS",
    cacher: "Memory"
});

broker.createService({
    name: "test",
    actions: {
        example: {
            cache: true, // Enable caching
            async handler(ctx) {
                // Artificial pause for demonstration purposes
                await new Promise((resolve) => setTimeout(resolve, 1000));

                return null; // Return null
            }
        }
    }
});

Possible solution

Instead of using a weak comparison against null in the cacher base (cachers/base.js#L303) we can strongly compare against undefined using !== undefined.
But I think the best solution would be to use a Symbol which is specific to moleculer. This way, we know nobody will be tempted to return it from inside their action.

However, I understand that this would be a breaking change (and I'm a bit saddened by this fact).

[icebob]

Could you create a PR? I think we can avoid the breaking change if we switch the old/new logic with a cacher option, e.g. storeNullValues: true

[icebob]

By the way, as I see, the value is stored, just not return it, right?

[Telokis]

By the way, as I see, the value is stored, just not return it, right?

Yes, from what I noticed, the value is properly stored and retrieved but the code I highlighted makes it seem like a cache miss and calls the action anyway.

Regarding the PR, I could do it if we come up with a good non-breaking solution, yes.

I thought it could be regarded as a breaking change for people who use custom cachers but I guess the configuration option fixes this issue.

[icebob]

Yeah, but in this case, the option name enableNullValues would be better because the storing is good. And as you mentioned, we should return a Symbol in get if it's not found in the cache and also check this symbol in the base middleware.

[Telokis]

Thanks for your answer. To make sure I understand:

• We add a configuration option enableNullValues to cachers/base.js#L29
• If enableNullValues is set to true
  • [Cacher] Return a specific Symbol instead of null to represent a cache miss.
  • [Middleware] When checking if a cache lookup missed, we check against the Symbol.
• If enableNullValues is false or undefined
  • [Cacher] We return null as we do now.
  • [Middleware] We weakly check against null to detect cache misses.

Is this correct?

[icebob]

Yeah, you are correct. Could you make a PR? Of course, need to add relevant tests, as well.

[intech]

@Telokis any news?

[Telokis]

@intech Hi! No, I didn't work on/with Moleculer since around then. I still need to properly finish my PR #829 and won't consider tackling this issue until that PR is done.

[intech]

@intech Hi! No, I didn't work on/with Moleculer since around then. I still need to properly finish my PR #829 and won't consider tackling this issue until that PR is done.

@Telokis thanks for the answer. I have now submitted for consideration changes to the serialization algorithm in order to solve 3 urgent problems at once, including yours. Therefore, I wanted to warn you, before you start work, let me know in the comments about it. Perhaps we will already have another solution ready.

[Telokis]

@intech Hi! No, I didn't work on/with Moleculer since around then. I still need to properly finish my PR #829 and won't consider tackling this issue until that PR is done.

@Telokis thanks for the answer. I have now submitted for consideration changes to the serialization algorithm in order to solve 3 urgent problems at once, including yours. Therefore, I wanted to warn you, before you start work, let me know in the comments about it. Perhaps we will already have another solution ready.

@intech Ok, seems like a good idea, thank you for letting me know about it!

[icebob]

I've added a missingResponse cacher option. The cachers return the value of it if the key is not found. By default, the value is undefined, so it enables to store null values, as well. If you would like to store undefined values, then you can change it to a symbol, e.g.: missingResponse: Symbol("MISSING").