## About the changes
This adds handling of unhandled rejected promises at the process level,
so nodejs doesn't panic.
Also, adds tests around processing metrics and a refactor from previous
pr: #11010
Allows the `checkLatestVersion` function in the `VersionService` to
accept an optional `instanceInfo` parameter. If provided, and if the
promise returns a value that is truthy, then it will add `instanceInfo`
to the versionPayload.
The license key may not contain a plan or a customer name, and while it
definitely won't contain a client id, it has been requested that we
report `self-hosted` as the client ID (will be handled in enterprise).
Adding a second, optional parameter seemed to be the most backwards
compatible way of doing this rather than changing the established method
/ callback types.
## About the changes
Properly awaits all submitted promises, preventing the node's main
process from seeing rejected & unawaited promises.
What's going on?
- The bulk metrics handler pushes `registerBackendClient` promises into
promises.
- The next step (`clientMetricsEnvBulkSchema.validateAsync`) throws for
invalid metrics (e.g., `appName: null`), so we jump to catch and return
400.
- Because the code never reaches `Promise.all(...)`, the previously
spawned promises are never awaited. Node later detects the rejected
`registerBackendClient` promise as an **unhandled rejection** and
crashes the process. If that promise hadn’t been rejected, there’d be no
crash, but with invalid input, it does reject.
- **Fix:** always await the spawned tasks (using `Promise.allSettled`)
so every rejection is observed, even when validation later throws.
The `create` and `update` role methods used to blindly accept any
incoming permissions, but if the permissions don't exist in the
database, then the database would throw, yielding a 500 error to the
user.
To fix this, we can validate that all the permissions exist before we
try to add the incoming permissions.
The http error only manifests in enterprise, but the fix requires
modifying the access service. Therefore, I've added the tests to the
access service too, such that if you break something, then you don't
need to wait for it to propagate to enterprise.
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
TypeScript throws `TS7056` because the schema object becomes too large
for the compiler to fully serialize when using deep literal inference.
Splitting the components object and explicitly reconstructing the type
prevents the error while preserving correct type inference.
Adds a test to ensure that the `getAll` method of the flag resolver
doesn't return the disabled variant if a flag is defined as a boolean in
the settings.
We have some places in the UI where we check `if
(uiConfig.flags.<flagname>) {...}`. If one of these flags were suddenly
returned as the disabled variant instead of `false`, then it'd be
impossible to turn it off.
As such, to maintain backwards compatibility and adhere to the principle
of least surprise, I'd like to add this test to ensure this doesn't
change going forward.
Fixes a bug / uncovered edge case in the flag resolver in Unleash:
If a local experiment was defined as false (the typical default value),
then that flag could only ever be returned as a boolean from the
`ui-config` endpoint. In other words, even if the external resolver has
a variant for that flag, the UI would never get the variant.
The fix is to not just check `isEnabled` for false flags, but instead:
- use `getVariant`
- then check `variant.enabled` (in which case we have a variant and can
return it)
- else check `variant.feature_enabled`, falling back to `isEnabled` only
if `feature_enabled` is null/undefined.
