Skip to content

fix: return error when config file cannot be parsed - #574

Open
Sarthak-Shreshtha01 wants to merge 3 commits into
microcks:masterfrom
Sarthak-Shreshtha01:fix/issue-572
Open

fix: return error when config file cannot be parsed#574
Sarthak-Shreshtha01 wants to merge 3 commits into
microcks:masterfrom
Sarthak-Shreshtha01:fix/issue-572

Conversation

@Sarthak-Shreshtha01

@Sarthak-Shreshtha01 Sarthak-Shreshtha01 commented Oct 8, 2026

Copy link
Copy Markdown

ReadLocalConfig and ReadLocalWatchConfig now return an error when the config file has invalid YAML, instead of an empty config. Before, login treated the broken file as a fresh install and overwrote all other contexts. The related tests pass, including the new TestReadMalformedConfig.

Fixes #572

@Sarthak-Shreshtha01
fix: return error when config file cannot be parsed
75c0eb3 
Signed-off-by: Sarthak <sarthakshreshtha345@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 11:52
@Sarthak-Shreshtha01 Sarthak-Shreshtha01 mentioned this pull request Oct 8, 2026
Open
Copilot AI reviewed Oct 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gyanranjanpanda

gyanranjanpanda commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hi @Sarthak-Shreshtha01, thank you for submitting this PR!

It looks like the review couldn't complete because your GitHub Copilot review quota was reached. Additionally, please make sure to check our contribution guidelines and this issue #559 regarding monthly PR limits before creating a new PR. Thanks again for contributing!

Harsh4902
Harsh4902 requested changes Oct 9, 2026
View reviewed changes
Comment thread pkg/config/localconfig.go
Comment thread pkg/config/localconfig.go
@Harsh4902 Harsh4902 added kind/bug Something isn't working component/cli and removed kind/bug Something isn't working labels Oct 9, 2026
@Harsh4902 Harsh4902 added this to the Next milestone Oct 9, 2026
@Sarthak-Shreshtha01
fix: use the centralized error-handling mechanism in ReadLocalConfig
5f239b9 
ReadLocalConfig and ReadLocalWatchConfig returned a plain fmt.Errorf
for a malformed config file instead of going through errors.Wrap like
the rest of pkg/*. Per documentation/error-handling.md, a malformed
local input file is errors.KindUsage, so wrap it that way and assert
the kind in TestReadMalformedConfig.

Signed-off-by: Sarthak <sarthakshreshtha345@gmail.com>
@Sarthak-Shreshtha01

Sarthak-Shreshtha01 commented Oct 12, 2026

Copy link
Copy Markdown
Author

@Harsh4902 Both read errors in ReadLocalConfig and ReadLocalWatchConfig now go through errors.Wrapf(errors.KindUsage, ...) from pkg/errors (commit 5f239b9). I used KindUsage to match the other invalid-local-config errors, such as the ones in logout and context. The config tests still pass.

@yada

yada commented Oct 14, 2026

Copy link
Copy Markdown
Member

Tested this branch on a build of the PR head.

  • go test ./pkg/config/ passes and go vet is clean.
  • Repro from bug: YAML parse error in config is silently swallowed, login overwrites all other contexts #572 (tab-indented config with an other-ctx context): microcks context now fails with reading config <path>: yaml: line 2: found character that cannot start any token. Before this change it printed "no contexts defined".
  • microcks login no longer overwrites the file. The config was byte-identical afterwards (the login itself only failed with connection refused, since no server was running).
  • One question: the new test asserts KindUsage (exit 2), but the CLI exits 14 (Environment) on that error in my run. Could you check which caller re-wraps it, or adjust the test or the kind so they agree?
  • policy-guards and build-verify did not run on this PR. Closing and reopening it would trigger them.

Disclosure: this comment was drafted with AI assistance and the results were run locally.

@yada yada added the kind/bug Something isn't working label Oct 15, 2026
@Sarthak-Shreshtha01
fix: report a malformed config as an environment error
22f6bb1 
Most commands already wrap config read errors as KindEnvironment, so the
same bad file exited with 2 from logout, stop and login but 14 from
context, import and start. Use KindEnvironment in ReadLocalConfig and
ReadLocalWatchConfig so every command exits the same way.

Signed-off-by: Sarthak <sarthakshreshtha345@gmail.com>
@Sarthak-Shreshtha01

Sarthak-Shreshtha01 commented Oct 15, 2026

Copy link
Copy Markdown
Author

@yada context, import and start re-wrap the read error as KindEnvironment, while logout, stop and login return it as it is. So the same bad file exited with 2 or 14 depending on the command. ReadLocalConfig and ReadLocalWatchConfig now use KindEnvironment (22f6bb1), so every command exits with 14, and the test checks that.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@Harsh4902 Harsh4902 Harsh4902 requested changes
@lbroudoux lbroudoux Awaiting requested review from lbroudoux lbroudoux is a code owner
@yada yada Awaiting requested review from yada yada is a code owner
+1 more reviewer
@Vaishnav88sk Vaishnav88sk Vaishnav88sk approved these changes
Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

component/cli kind/bug Something isn't working

Projects

None yet

Milestone

Next

Development

Successfully merging this pull request may close these issues.

bug: YAML parse error in config is silently swallowed, login overwrites all other contexts

6 participants

@Sarthak-Shreshtha01 @gyanranjanpanda @yada @Harsh4902 @Vaishnav88sk