Repository navigation
Return non-zero exit code when commands fail - #1992
Open
waldekmastykarz wants to merge 6 commits into
Open
waldekmastykarz wants to merge 6 commits into
waldekmastykarz wants to merge 6 commits into
Conversation
`cert ensure`, `config new` and `mocks from-http-responses` logged errors but exited with code 0. They now return 1 on failure. `mocks from-http-responses` also returns 1 when any input file fails to parse (still writing mocks for the files that succeeded). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fcec9529-b9a7-41f7-9e1d-fdb9fc72cd4b
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Logged certificate trust failures and non-throwing parse errors can still produce exit code 0.
2 open issues
What changed in this PR
Improves Dev Proxy command exit codes so scripts can detect failures.
Changes:
- Returns failure codes for certificate and configuration exceptions.
- Tracks mock input failures while preserving successfully generated mocks.
- Adds process-level success and failure tests.
| File | Description |
|---|---|
| DevProxy/Commands/ConfigCommand.cs | Propagates configuration creation exit codes. |
| DevProxy/Commands/CertCommand.cs | Returns failure when certificate trust throws. |
| DevProxy.Plugins/Mocking/MockResponsePlugin.cs | Reports missing inputs and caught parsing failures. |
| DevProxy.Integration.Tests/CommandExitCodeTests.cs | Tests configuration and mock command exit codes. |
Review effort: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fcec9529-b9a7-41f7-9e1d-fdb9fc72cd4b
Preserve generated mock contents while reporting malformed headers and JSON bodies as failures. Addresses review feedback on dotnet#1992. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Return helper results to distinguish installation failure from success, while preserving best-effort automatic trust. Update the Trust contract and cover failure exit codes. Addresses dotnet#1992 review feedback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes propagate failures correctly, preserve existing fallback behavior, and include targeted regression coverage with no unresolved blocking findings.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1959
cert ensure,config new,config getandmocks from-http-responseslogged an error but exited with code 0, breaking scripts and agents that rely on exit codes. They now exit with1on failure (matchingcert remove).cert ensure: returns 1 when trusting the certificate throwsconfig new: returns 1 when creating the file failsconfig get: returns 1 when the config isn't found in the samples repo or the download failsmocks from-http-responses: returns 1 for missing args / no matching files, and when any input file fails to parse (mocks for successfully parsed files are still written)Tests
Added
CommandExitCodeTests(process-level, inDevProxy.Integration.Tests) covering success and failure forconfig newandmocks from-http-responses, and failure forconfig get(routed through an unreachable proxy, so no network access is needed). The failure tests fail onmainand pass with this change. All test suites pass.