ui: fix errors thrown by 0.toNumber()

[sjbarag]

When generating JS protobuf bindings, protobufjs's pbjs CLI accepts a --strict-long (or the more modern --force-long) flag with the following description:

Enfores the use of 'Long' for s-/u-/int64 and s-/fixed64 fields.

This uses the long package on NPM¹, which it expects to be present when one of those clients is used at runtime. If long isn't available when pbjs is executed, it falls back to generating number-based fields where possible. Long is available in all consumers of crdb-protobuf-client, and protobufjs declares long as one of its few build-time and run-time dependencies, so nothing looked alarming.

For each field with a type that might be represented by a Long (a heuristic influenced by --strict-long), that field is marked as "long" if and only if the protobufjs-internal util.Long is non-null². That util.Long value is set via the use of the @protobufjs/inquire package instead of a standard require³ though, since the former silently returns null instead of throwing an error⁴. This, again, is not alarming on its own because protobufjs depends directly on long.

The complication comes in with rules_js' use of the pnpm visibility rules, which are very strict when it comes to JS package dependencies. Put simply: packages only have access to the dependencies they directly declare, not transitive dependencies ("phantom dependencies")⁵. Since @protobufjs/inquire is a separate package with no direct dependencies of its own⁶, Bazel lays out a node_modules tree where @protobufjs/inquire has no access to the long package. Since inaccessible packages are indistinguishable from nonexistent packages and inquire is intended to be silent, it returns null. The pbjs CLI then assumes Long isn't installed and falls back to number-based fields where a Long isn't strictly required.

When combined with protobuf's zero-value, this caused several cases of '0.toNumber is not a function' JS errors at runtime⁷. We requested a Long, we support Longs, but a client was silently generated without respecting --strict-long.

Like with most other phantom dependencies, use pnpm.packageOverrides to declare that @protobufjs/inquire does in fact depend on Long.

Fixes: #106318
Release note: Observability pages no longer crash when they encounter zeros (e.g. a session with no memory allocated).

Footnotes

1. https://www.npmjs.com/package/long ↩

2. https://github.com/protobufjs/protobuf.js/blob/918ff014efe19f3eb43195ae3d71f7aeb3fcdd73/src/field.js#L153-L157 ↩

3. https://github.com/protobufjs/protobuf.js/blob/918ff014efe19f3eb43195ae3d71f7aeb3fcdd73/src/util/minimal.js#L163-L167 ↩

4. https://github.com/protobufjs/protobuf.js/blob/918ff014efe19f3eb43195ae3d71f7aeb3fcdd73/lib/inquire/index.js#L4-L17 ↩

5. https://docs.aspect.build/rules/aspect_rules_js/docs/pnpm/#hoisting ↩

6. https://github.com/protobufjs/protobuf.js/blob/918ff014efe19f3eb43195ae3d71f7aeb3fcdd73/lib/inquire/package.json ↩

7. https://github.com/cockroachdb/cockroach/issues/106318 ↩

[cockroach-teamcity]

This change is 

[zachlite]

LGTM. Looks like there was test fallout.

[sjbarag]

@maryliag can you or someone on your team gut-check the test change here? Now that Longs are always produced, looks like there was one small test update required.

[maryliag]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @sjbarag)

[sjbarag]

Thanks for the reviews y'all!

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)