fix(varlock): record missing dataType as a ResolutionError - #1069
fix(varlock): record missing dataType as a ResolutionError#1069hassaans wants to merge 1 commit into
Conversation
ConfigItem.resolve threw `expected dataType to be set` when finishLoad() bailed early (invalid source or plugin load failure) and the item was then resolved via a process.env override. resolveEnvValues() does not await resolveItem() and uses a dead _reject, so that throw became an unhandledRejection while the returned promise hung forever. Record a ResolutionError on the item instead so callers can fail cleanly. Reproduces the crash hq-cli saw as Sentry 7687037996.
There was a problem hiding this comment.
Important
The new regression test fails repository lint because its Promise executor returns a timer handle.
Reviewed changes Reviewed the missing data type resolution fallback and its regression coverage.
- Resolution fallback:
ConfigItem.resolve()now records aResolutionErrorwhen loading ended before an item received a data type, allowingresolveEnvValues()to settle instead of hanging on an unhandled assertion. - Regression coverage: The new test reproduces an invalid sibling source plus override and verifies settled resolution and item error reporting.
azure/gpt-5.6-sol | 𝕏
| ); | ||
| const result = await Promise.race([ | ||
| settled, | ||
| new Promise<'STILL_PENDING'>((r) => setTimeout(() => r('STILL_PENDING'), 200)), |
There was a problem hiding this comment.
Changed-file ESLint fails here because this expression-bodied Promise executor returns the Timeout from setTimeout, violating no-promise-executor-return; this will make lint CI fail. Wrap the executor body in braces and call setTimeout without returning it.
| new Promise<'STILL_PENDING'>((r) => setTimeout(() => r('STILL_PENDING'), 200)), | |
| new Promise<'STILL_PENDING'>((r) => { | |
| setTimeout(() => r('STILL_PENDING'), 200); | |
| }), |
|
Thanks for this. Will get it sorted asap. |
|
Thanks for the report and the repro, that was a good catch and the diagnosis is exactly right. We went a different direction on the fix though, in #1072.
So #1072 catches it a level earlier instead:
Two questions, if you don't mind: How are you using varlock in hq-cli? I'd like to make sure the fix actually covers your path. The crash reaches Heads up that Closing this in favor of #1072, but the fix is yours in substance. Thanks again. |

Summary
ConfigItem.resolvethrowsError: expected dataType to be setwhenfinishLoad()returns early (any invalid source, or a pluginloadingError) and the item is then resolved because its key is also inprocess.env/overrideValues.EnvGraph.resolveEnvValues()does not awaitresolveItem()and uses a dead_reject, so that throw becomes an unhandledRejection while the promise the caller awaits hangs forever.This is the crash hq-cli users hit as Sentry
7687037996(and the same surface as #629 / #720 / #758). No released version removes the assertion.Change
Record a
ResolutionErroron the item and return, matching every other resolve failure.resolveEnvValues()can then settle; callers seeitem.errorsinstead of a floated assertion.Test
packages/varlock/src/env-graph/test/unprocessed-item-resolve.test.ts— valid.env.schema+ malformed.env.local+ override. Without the fix,resolveEnvValues()hangs and the assertion is unhandled. With it, the promise settles and the item carries aResolutionError.Does not change successful resolution.
Related: https://github.com/indigoai-us/hq-cli (consumer workaround while this lands).