fix(config): improve auth config errors and print config warnings before failures#11306
fix(config): improve auth config errors and print config warnings before failures#11306michkot wants to merge 2 commits into
Conversation
|
I don't think this solution is correct. The issue is that the env variable is not being replaced in the value here: |
|
// done with edits
The existing code already treats the situation, but it treats it as warning -- I am okey with changing that existing paradigm, my default was to retain it (once I found that this logic exists). Pre-existing tests here: The logic for warning is |
@zkochan I am happy to close this and make another PR where the existing warning semantics is changed to error if that's what you think should be done? |
Summary
fixes #11298, this MR improves the config/auth failure path for unresolved credential placeholders and makes sure already-collected config warnings are still printed when config loading throws.
What changed
_auth/_passwordvalues reach credential parsing, instead of surfacing a low-level decode failure.finallypath, so warnings are emitted both on successful config loading and on config-loading failure.Why
#11298 is the motivation for this MR and contains the full problem statement and reproduction, so this description intentionally does not repeat that analysis here. The key point for this change is that the warning path and the fatal error path were previously disconnected: warnings could be collected during config loading, but they were only printed if
getConfig()completed successfully.The warning-handling change uses a caller-provided warnings array because
@pnpm/config.reader.getConfig()already has multiple callers with different output behavior. The mainpnpmCLI prints warnings, while another direct caller does not print them. Keeping warning collection inconfig.readerand warning presentation in the caller preserves that existing boundary, while still allowing the CLI to print collected warnings even when config loading throws.Important note
If the long-term decision is that this kind of invalid config should not stop the CLI, the warning-printing part of this MR is still relevant.
In that case, the follow-up should not rely on throwing around the
atob()path and recovering elsewhere. Instead, auth parsing would need a different policy that treats unresolved / invalid credential input as a warning-only condition and yields no credentials rather than throwing.