Data: TypeError: Cannot read property 'getRandomValues' of undefined

[williamweckl]

Reproduction

model() {
    return this.store.createRecord('profile');
}

Description

Creating a new record to the store raises the error TypeError: Cannot read property 'getRandomValues' of undefined when page is loaded with fastboot.

Probably this commit has introduced the issue: https://github.com/emberjs/data/commit/576a02a0a7279849e82347c2d459d1ef1295807c#diff-ddf3f6ae6821e1502f6a41d26c52c8a8R13

Versions

Ember cli: 3.13.1
Ember data: 3.13.1
Ember source: 3.12.0

[rwjblue]

Thank you for reporting! Definitely seems like a bug...

[rwjblue]

Doesn't seem like that specific change (576a02a#diff-ddf3f6ae6821e1502f6a41d26c52c8a8R13) did anything but tweak formatting of that line. Seems like the real culprit was the original identifiers PR (#6247).

In the current code, we have this:

https://github.com/emberjs/data/blob/835fd28090e6ef61a8497c5cc6857cebc7167df3/packages/store/addon/-private/identifiers/utils/uuid-v4.ts#L12-L15

In FastBoot window is defined, window.crypto is undefined, and window.msCrypto is undefined.

We then call CRYPTO.getRandomValues unconditionally in rng:

https://github.com/emberjs/data/blob/835fd28090e6ef61a8497c5cc6857cebc7167df3/packages/store/addon/-private/identifiers/utils/uuid-v4.ts#L18-L23

Which is called unconditionally in the default export of uuid-v4.ts:

https://github.com/emberjs/data/blob/835fd28090e6ef61a8497c5cc6857cebc7167df3/packages/store/addon/-private/identifiers/utils/uuid-v4.ts#L61-L69

Which is ultimately called from the default identifier generator (which is used during store.createRecord if the user hasn't customized it):

https://github.com/emberjs/data/blob/576a02a0a7279849e82347c2d459d1ef1295807c/packages/store/addon/-private/identifiers/cache.ts#L61-L70

@runspired - Mind giving a quick brain dump on how this is supposed to be working in FastBoot? I don't see how CRYPTO (in FastBoot) can ever be something other than undefined, and therefore how store.createRecord _could_ work there...

[ballPointPenguin]

I wonder if this is something that should be improved in fastboot itself, ie incorporating nodejs 'crypto' module and making it available, or if it is more the responsibility of ember-data to provide that.

[rwjblue]

It is definitely possible for ember-data to require('crypto') if that is what we want to do (we'd have to use FastBoot.require though). I'm just not sure what the original design was for this, so wanted to check in with @runspired first...

[runspired]

We would want to require node crypto JIT. Before identifiers, Users shouldn’t have been using createRecord in fastboot though, and it still feels weird to me, although I don’t see much of an alternative pattern in today’s shoebox pattern

[rwjblue]

Doesn't really seem like a shoebox thing to me, calling store.createRecord seems super common for "new" routes why wouldn't you expect that to happen in FastBoot scenarios?

[runspired]

Because there's no way to stash the state for rehydration today. Identifiers at least makes it possible to rehydrate that state, still feels weird to be rehydrating "mutations" in this way.

[rwjblue]

Because there's no way to stash the state for rehydration today.

It is also not about storing partial mutations across the SSR -> client boundary. The pattern I'm talking about is where you'd have a posts.new route that does return this.store.createRecord('post'), and its template would use {{input value=model.title}}. This pattern seems perfectly fine to me, and shouldn't be considered "bad".

[rwjblue]

FWIW, I discussed this more with @runspired in chat. We were talking about different kinds of mutations and completely talking past each other above. Sorry about that @runspired.

RE: this specific issue, we are definitely agree that we need to fix (likely by way of FastBoot.require('crypto').randomBytes in fastboot scenarios). The quick fix will add that code in runtime, but a more thorough fix should attempt to only ship that code in fastboot scenarios.

[rwjblue]

I took a first stab at this over in https://github.com/emberjs/data/pull/6568. I'm not super tied to the specific implementation and if we don't want to go that route we can throw away the implementation commit and just use the test commit.

[williamweckl]

Thank you guys for the amazing job done!