-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Parsing RFC DateTime string doesn't work with 4.5.1 add-on but works with npm installed library #288
Comments
I cannot reproduce this on my clean dev system.
This should matter. |
However I found a bug, that |
Just to be clear. When both openhab-js and openhab-rules-tools is installed via When I remove openhab-js from node_modules and change the add-on settings to "use built in library" the error is generated. I do a restart of OH in between. So openhab_rules_tools picking up npm library shouldn't be happening. And even if it did, should that mean no error? It's odd that you cannot reproduce it as it's not just me who is seeing this. I've had several report similar problems on my rule templates.
That certainly could be coming into play here, but that doesn't explain how the error appears and disappears even though the same openhab-js version is in use. I'd like to understand what could possibly be driving this a it will help me support my rule template users and library users if I can figure out why there is this disconnect. I'll run a few more tests later today and report back if it makes sense. Maybe I wasn't as careful as I thought and I did forget to remove the library. |
I have re-tested with exactly the the Item state from your error message above and it works fine when "Use built-in library" is enabled. I have also tested passing a DateTime Item directly with my fix from #290 in place: // Test 1
var t = time.toZDT('2023-08-09T16:22:13.585-06:00[SYSTEM]')
// Test 2
var i = items.test_datetime
i.postUpdate(t)
t = time.toZDT(i)
To explain this, I have to go a bit into the detail. The JS Scripting add-on comes with two version of openhab-js included, both are webpacked:
"Do not use built-in library" option basically opts out of the cached version to allow loading openhab from the file system (means npm installed) - in none is there, the included version (1) is used. |
Very weird. I'm fixing some bugs in my rule templates but retesting this is next on my list.
Where I'm still confused is if there is no openhab-js on the file system at all, shouldn't openhab_rules_tools use the only library available which is built into the add-on? And given that, if there is an error, shouldn't that error always occur whether it's loaded from openhab_rules_tools or in a rule directly using Similarly, if the version of openhab-js on the file system is the same version as in the add-on (as reported by The root behavior I'm seeing is that |
I haven't had a chance to run more tests yet but I had yet another user encounter the problem. This time with a ZonedDateTime, not a DateTime Item. See https://community.openhab.org/t/openhab-rule-tools-timermgr-exception-with-zoneddatetime/148766/2 Interestingly they were able to make it work by adding
to the top of the script action. I wonder if there is something weird going on with the way the libraries are injected? |
I've been able to gather a bit more data. The problem is when I pass a js-joda ZonedDateTime to The problem only occurs when Given the following code run using the built in library:
produces:
The same code, run with openhab-js installed on the file system and not using the built in library does not produce any errors at all. So there is something about I've updated my rule templates to pass Hopefully this gives you enough to reproduce it? I know it's not just on my system because I've had several of my rule template users run into the problem. The following is where the problem lies I think, at least code wise.
Why would |
Out of interest: Can you please keep openhab from npm installed and set the add-on to use the internal version? Does the error occur then?
If npm rm openhab does not remove js-joda, this probably is a npm issue or by design. I don’t know. Reading through your stacktrace (the interesting part is at I have the same suspect for the root cause of the problem, you should be able to add logging there. Just add your logging, then |
With the library installed on the file system but the add-on configured to use the internal version the error still occurs. However, the line number and path to the file for
When I switch to not use the webpacked library there is no error. So it seems the error always occurs when the add-on is configured to use the embedded library even though it appears that OHRT is pulling the library from the file system. I'll come back with the results of adding some logging later today when I get a chance to do it. I've updated my rule templates temporarily to call |
…stanceof checks don't work (#312) Fixes #288. When `time.toZDT` got a `time.ZonedDateTime` passed in from an external library, it failed to recognize that it in fact got a `time.ZonedDateTime` because instanceof checks were not working due to the webpack settings for the injection caching. Also fixes `utils.isJsInstanceOfJava` not working and renames `utils.isJsInstanceOfJava` to `utils.isJsInstanceOfJavaType`. --------- Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@rkoshak I have fixed that in #312. The problem was quite complicated, it was caused because some instanceof checks were not reliably working in the webpacked version for injection caching, because webpack is throwing away constructor names. I will now create a PR to get this into 4.1.0. BTW: I've sent you a LinkedIn request, can you please accept it? |
I'm very rarely on LinkedIn. I should probably shut my account down. In the mean time I'll go accept it. |
Expected Behavior
Passing the
state
of a DateTime Item totime.toZDT()
gets parsed into atime.ZonedDateTime
.Current Behavior
When using the library built into the add-on (4.5.1 according to
utils.OPENHAB_JS_VERSION
) the following exception occurs:However, installing the 4.5.1 through
npm
and switching the add-on to not use the built in library it works just fine.Admittedly these are being called from the openhab_rules_tools library (which you can see from the stack trace) but that shouldn't matter, should it?
Maybe the problem is that
openhab
and@joda-js
are still there in the node-modules? Should that matter? (I need to run the test if the error occurs if @joda-js isn't present but don't have time today, I'll come back tomorrow).Possible Solution
I'm not even sure if this belongs here or as an issue on the add-on.
Steps to Reproduce (for Bugs)
time.toZDT(items.SomeDateTimeItem.state)
and run itContext
This is impacting users of my rule templates. I've had more than one report this problem with Time Based State Machine and Threshold Alert.
Your Environment
The text was updated successfully, but these errors were encountered: