Unhandled promise rejections from uncaught background init (`handleWhenReady`)

[dermotduffy]

Importing the package kicks off an async init (handleWhenReady in dist/index.js) that is fire-and-forget: it is never awaited or .catch()-ed, and it self-reschedules via setTimeout. Any throw in that background chain therefore surfaces as an unhandled promise rejection in the consuming app. This happens in a normal browser, not just in tests:

1. Floating registry fetches. In the ready handler, window.haNunjucks[registry].fetchRegistry(ha.hass) is called without await/.catch. fetchLabelRegistry does await connection.sendMessagePromise(...), so a transient WebSocket hiccup (e.g. an HA reconnect) rejects unhandled.
2. Check/use mismatch. The readiness predicate guards with ha?.hass?.connected, but the handler then reads ha.hass unguarded. If the home-assistant element or its hass goes away in the await gap (dashboard reload/re-render), it throws.
3. Unguarded document. Both the handler and the predicate call document.querySelector('home-assistant') with no guard, so import in any non-DOM/torn-down context (SSR, unit tests) throws document is not defined from the retry timer.

Repro

Import the module in a context where no connected <home-assistant> is present (a jsdom/node test is the simplest), then let the environment tear down. A pending handleWhenReady retry fires and throws:

ReferenceError: document is not defined
 ❯ dist/index.js  (handler)
 ❯ handleWhenReady dist/helpers.js
 ❯ Timeout._onTimeout dist/helpers.js

Suggested fix (behavior-preserving)

• .catch() the top-level handleWhenReady(...) call and the floating fetchRegistry(...) promises.
• Guard the DOM access (typeof document !== 'undefined') and have the handler use the same optional chaining as the predicate.
• Optionally, expose an explicit/lazy init (or skip the global setup when hass is passed to renderTemplate) so the library is safe to import outside a live browser.

Confirmed present and identical in the latest release (1.7.6).

[dermotduffy]

Concrete example of the issue this caused: https://github.com/dermotduffy/advanced-camera-card/actions/runs/28276763971/job/83784798179?pr=2549

PS: Thank you so for the work on this library! It is fantastic to have such a fundamental building block be made available to other developers, and very much appreciated. I know a passion project when I see one. Thank you.

[Nerwyn]

Thanks for the report, and using this library in your card. I actually recently switched all my security cameras to Frigate and use advanced camera card to view their event history.

1. Floating registry fetches. In the ready handler, window.haNunjucks[registry].fetchRegistry(ha.hass) is called without await/.catch. fetchLabelRegistry does await connection.sendMessagePromise(...), so a transient WebSocket hiccup (e.g. an HA reconnect) rejects unhandled.

I'll add a a try/catch so errors are properly logged, and delete window.haNunjucks so if there is an error so if it's imported again it can be retried.

2. Check/use mismatch. The readiness predicate guards with ha?.hass?.connected, but the handler then reads ha.hass unguarded. If the home-assistant element or its hass goes away in the await gap (dashboard reload/re-render), it throws.

Have you seen this cause an error Home Assistant? It sounds highly unlikely that this would happen since the handler isn't called until the hass object is defined and connected. It disappearing or disconnecting is highly unlikely and an error scenario. I can add a check and throw an error if the connection disappears anyway.

3. Unguarded document. Both the handler and the predicate call document.querySelector('home-assistant') with no guard, so import in any non-DOM/torn-down context (SSR, unit tests) throws document is not defined from the retry timer.

True, but this package shouldn't be used in server side scenarios and document can be mocked for unit tests. This package has unit tests and mocks the DOM using global-jsdom in this fixtures file and adds a test home-assistant element to it. If it's causing issues for your unit tests, maybe you can try using the same method?

[dermotduffy]

Thanks for the report, and using this library in your card. I actually recently switched all my security cameras to Frigate and use advanced camera card to view their event history.

Nice!

I'll add a a try/catch so errors are properly logged, and delete window.haNunjucks so if there is an error so if it's imported again it can be retried.

Sounds good.

Have you seen this cause an error Home Assistant? It sounds highly unlikely that this would happen since the handler isn't called until the hass object is defined and connected. It disappearing or disconnecting is highly unlikely and an error scenario. I can add a check and throw an error if the connection disappears anyway.

No, I have not, so this seems like a narrow race condition possibility. To be clear, whilst the issues highlighted here seem like they could happen in a real Home Assistant instance, I ran into in a testing context.

True, but this package shouldn't be used in server side scenarios and document can be mocked for unit tests. This package has unit tests and mocks the DOM using global-jsdom in this fixtures file and adds a test home-assistant element to it. If it's causing issues for your unit tests, maybe you can try using the same method?

Yes. I'm using this right now: https://github.com/dermotduffy/advanced-camera-card/blob/dev/tests/test-utils.ts#L770 .

Summary: It's really the first issue, the "hidden floating promise", that feels like it might surprise people.

[Nerwyn]

I've created a 1.7.7 beta version with this fix. Here's the diff.

I do see that you're on the last minor version, so you'll have to update to the latest patch version to fix this. In 1.7.0 there was a breaking change for developers which requires node polyfills for some new functions that have to be included in your bundler config. See here in the README. It looks like you're using Rollup, which has a plugin you can add to fix this.

[dermotduffy]

@Nerwyn Thank you! Yeah, hanging back on the last minor was intentional -- I'm not sure (yet at least) that it's worth the bloat of the polyfills, as I highly doubt Advanced Camera Card users will want the new functions added, so I was going to "hang back" until someone requests one of the new features.

That's my problem though -- thank you again for the fix!

[Nerwyn]

What if I remove that bundler polyfill requirements? It turns out I can use jscrypto instead for the hashing functions and it seems to work with that bundler polyfill removed. It'll be included in this patch and I'll update the docs and release notes. Diff here.

[dermotduffy]

Well, I'm just one developer vs a whole community -- so take this with a pinch of salt of course.

In my case, the bundle was estimated to be +46KB gzip'd difference with head vs v1.6.2 (which I pinned at), roughly doubling the cost of the template engine as a whole. My users are likely doing much more simple template operations that anything requiring crypto, so even though (in my case) I lazy load the whole thing, the payoff didn't seem clear for 46KB even in that case. So I was just going to wait until either there was a clear compelling reason to have to upgrade, OR a user requests one of the missing functions.

Now if 46K was 5K or 10K, I wouldn't have cared at all. So, depends on the bundle difference...

[Nerwyn]

Ah I didn't realize that 1.7.0 made ha-nunjucks so much larger. Funnily enough, I was working on some JS bundle optimization stuff at work recently so I can look into what the big offenders are and if they can be replaced.

[Nerwyn]

Oof the jscrypto package I just added is the biggest offender. I'll have to see how that looks after bundling in one of my projects since this doesn't account for minification or tree-shaking.

1.9M    node_modules/jscrypto
1.8M    node_modules/nunjucks
376K    node_modules/d3-array
272K    node_modules/home-assistant-js-websocket
232K    node_modules/stream
232K    node_modules/d3-time-format
204K    node_modules/ts-py-datetime
188K    node_modules/long
120K    node_modules/commander
108K    node_modules/d3-time
104K    node_modules/buffer
72K     node_modules/string_decoder
56K     node_modules/python-struct
52K     node_modules/asap
32K     node_modules/slugify
32K     node_modules/mersenne-twister
24K     node_modules/util-deprecate
24K     node_modules/internmap
24K     node_modules/base64-js
20K     node_modules/inherits
20K     node_modules/ieee754
20K     node_modules/a-sync-waterfall

[dermotduffy]

Ah I didn't realize that 1.7.0 made ha-nunjucks so much larger. Funnily enough, I was working on some JS bundle optimization stuff at work recently so I can look into what the big offenders are and if they can be replaced.

My $0.02: The ideal from my perspective, would be for ha-nunjucks to lazy load whatever is needed for a template. If a user uses a crypto function, and you dynamically fetch the crypto libraries, I wouldn't care about the bundle size at all in that case since they self-inflicted that with their choice of template, and since no-one else would be paying that cost (unless they also chose to use crypto). Similar approach could be used for other big ticket items in the featureset.

The downside is that lazy loading is async, so you'd need to expose some way to trigger the load, e.g:

// New
export function prepareTemplate(
	hass: HomeAssistant,
	str: string
): Promise<void> {
  // Dynamically loads whatever template features 'str' requires
  // Renders nothing
  // Doesn't resolve the promise until the lazy loading is complete.
}

// As-is today
export function renderTemplate(
	hass: HomeAssistant,
	str: string,
	context?: object,
	validate: boolean = true,
): string | boolean {
  // Existing template magic goes here!
}

Then on my end, I'd have:

const userTemplate = "{{ crazy_expensive_template() }}"
await renderer.prepareTemplate(hass, userTemplate)
const output = renderer.renderTemplate(hass, userTemplate)

[Nerwyn]

Custom card features with ha-nunjucks 1.6.2 - 383.4 kB minified, 100.2 kB tarballed.
Custom card features with ha-nunjucks 1.7.6 - 529.5 kB minified, 143.7 Kb tarballed.
Custom card features with ha-nunjucks 1.7.7 - 485.9 kB minified, 125.5 kB tarballed.

Oof, that's a big hit, but at least jscrypto is a big improvement over create-hash and the process polyfill.

One difficulty with lazy loading libraries for nunjucks is dynamically detecting and loading the templates without user (as in developers that use it like you!) input is a bit tricky. I could rewrite the nunjucks environment extension loading to only load a subset of common functions, but actually identifying when other templates are needed without developer input is difficult. I also don't want to necessarily go the route of not including all dependencies in the module and loading them from something like a CDN, as that would create a dependency on an online resource (even if it get's cached in the users browser after the first time it's fetched).

One alternative to all this I could do is use the browsers built in JS crypto module for hashing. The only problem there is that it's async. Nunjucks does support async filters but not globals, so I'd have to remove support for using these hash functions as function calls (md5('foobar')) but keep them as filters ('foobar' | md5). This wouldn't be the worst thing as these are fairly obscure template extensions and it may not be worth having full support for them for the sake of a greatly reduced bundle size.

[Nerwyn]

Nevermind about the async filters, they don't work with renderString without an async callback function.

[Nerwyn]

I got the custom-card-features bundle size down a little bit more to 458.1 kB minified and 123.8 kB tarballed after switching to @nobles/hashes. I'm going to keep trying alternate hashing modules later this week to see if I can find one that gets me near the ha-nunjucks 1.6 bundle size.