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

java.lang.NullPointerException when executing cache.[private|shared].get("...", () => null) #386

Open
SkyLined opened this issue Oct 2, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@SkyLined
Copy link

SkyLined commented Oct 2, 2024

Expected Behavior

The following code should output null to the console.

cache.private.remove("x"); // just in case it exists
console.log(cache.private.get("x", () => null));

Expected output:

2024-10-02 09:36:10.004 [INFO ] [nhab.automation.script.ui.4f5b80b923] - null

The documentation at https://www.openhab.org/addons/automation/jsscripting/#cache says

 <snip>
  .get(key, defaultSupplier) ⇒ Object | null
 <snip>

The defaultSupplier provided function will return a default value if a specified key is not already associated with a value.

The documentation for JSCache at https://openhab.github.io/openhab-js/JSCache.html says:

 <snip>
  get(key, defaultSupplier[opt]) → {*|null}
 <snip>

defaultSupplier function <optional> if the specified key is not already associated with a value, this function will return a default value

From this documentation there is no reason to assume a defaultSupplier cannot return null (e.g. () => null).

Current Behavior

A defaultSupplier function that returns null causes scripts to fail unexpectedly with a java.lang.NullPointerException. This is the actual output:

2024-10-02 09:34:32.207 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID '4f5b80b923' failed: java.lang.NullPointerException

The source code at https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/shared/ValueCache.java#L61 says:

 /**
   * Get a value from the cache or create a new key-value-pair from the given supplier
   *
   * @param key the key of the requested value
   * @param supplier a supplier that returns a non-null value to be used if the key was not present
   * @return the value associated with the key
   */
 Object get(String key, Supplier<Object> supplier);

Note that the supplier documentation mentions that it returns a non-null value. Apparently the code is not able to handle a default supplier that returns null and doing so triggers the exception.

Possible Solution

  1. Ideally, I'd like to see the java code modified to allow a supplier function to return null without triggering a NullPointerException. I wasn't able to immediately track down the source to try and submit a patch.
  2. If that is not an option, I'd like for the code to catch the exception and return null to work around this. That is assuming there is only one reason the code can throw this exception. This change would introduce a risk of returning null when there is another reason to throw this exception. I don't like taking such risks.
  3. Finally, if all else fails, the documentation should warn users that their function must never return null to avoid breaking their code. This is sub-optimal because it would require others to work around this problem in the code and any oversight on the side of the user would unexpectedly breaks their code with this exception.

Steps to Reproduce (for Bugs)

  1. Create a new rule that executes an inline JavaScript, then put the bellow code in the script:
 cache.private.remove("x"); // just in case it exists
 console.log(cache.private.get("x", () => null));
  1. Run the rule
  2. Check the console output; it should contain a message that simply says null but a java.lang.NullPointerException error message is shown instead.

Context

I created a function that uses cache to store values but initialize them to a value that could sometimes be null. Using a defaultSupplier that returns null when appropriate would be the simplest implementation for this. However, I found it broke the script. I need to work around this by using .exists(key) and having two different code paths to either get the cached value or get an initial value. The fragility of the defaultSupplier negates it's value; I'd be better off avoiding it so I never have to worry about running into this issue again.

Your Environment

(Tip: It would be nice if the bug report template contained instructions on where to find the version information, as I have no idea where to find the openhab-js version)

  • openHAB 4.2.1 (Release Version)
  • Java 17.0.12
  • OS Linux/6.1.21-v8+ (aarch64) (Raspbian bullseye)
@SkyLined SkyLined added the bug Something isn't working label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant