From 4f1634f1643a6e49453c59e1878c8071309f186e Mon Sep 17 00:00:00 2001 From: Pravin Barton <9560941+isc-pbarton@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:38:07 -0400 Subject: [PATCH] Improve error reporting (#462) --- CHANGELOG.md | 1 + Dockerfile | 2 +- cls/SourceControl/Git/Utils.cls | 73 ++- csp/gitprojectsettings.csp | 2 +- .../2026-07-21-notopen-error-reporting.md | 543 ++++++++++++++++++ ...26-07-21-notopen-error-reporting-design.md | 233 ++++++++ .../Git/Utils/GitLaunchError.cls | 125 ++++ 7 files changed, 972 insertions(+), 7 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-21-notopen-error-reporting.md create mode 100644 docs/superpowers/specs/2026-07-21-notopen-error-reporting-design.md create mode 100644 test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls diff --git a/CHANGELOG.md b/CHANGELOG.md index 36937b9a..e69e495b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed a bug where interop deployment manager would in some cases fail because it tried to load unrelated items from the repo (#977) +- Improved reporting of `` errors on the git executable, with an actionable message and diagnostics pointing to likely causes (#462) ## [2.17.0] - 2026-06-22 diff --git a/Dockerfile b/Dockerfile index e4e31642..849325ca 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE=containers.intersystems.com/intersystems/iris-community:2025.1 +ARG BASE=containers.intersystems.com/intersystems/iris-community:2026.1 FROM ${BASE} diff --git a/cls/SourceControl/Git/Utils.cls b/cls/SourceControl/Git/Utils.cls index fc21b519..348fb1f4 100644 --- a/cls/SourceControl/Git/Utils.cls +++ b/cls/SourceControl/Git/Utils.cls @@ -2175,11 +2175,22 @@ ClassMethod RunGitCommandWithInput(command As %String, inFile As %String = "", O set env("XDG_CONFIG_HOME") = ##class(%File).ManagerDirectory() set returnCode = $zf(-100,"/ENV=env... "_baseArgs,gitCommand,newArgs...) } catch e { - if $$$isWINDOWS { - set returnCode = $zf(-100,baseArgs,gitCommand,newArgs...) - } else { - // If can't inject XDG_CONFIG_HOME (older IRIS version), need /SHELL on Linux to avoid permissions errors trying to use root's config - set returnCode = $zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...) + // /ENV injection failed (older IRIS); retry without it. + try { + if $$$isWINDOWS { + set returnCode = $zf(-100,baseArgs,gitCommand,newArgs...) + } else { + // If can't inject XDG_CONFIG_HOME (older IRIS version), need /SHELL on Linux to avoid permissions errors trying to use root's config + set returnCode = $zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...) + } + } catch retryErr { + throw ..GitLaunchError(retryErr) + } + // On Linux, the /SHELL retry absorbs a failed git launch into a shell exit + // code (127 = command not found, 126 = found but not executable) instead of + // throwing , so the launch failure must be detected here instead. + if '$$$isWINDOWS && ((returnCode = 127) || (returnCode = 126)) { + throw ..GitLaunchFailedError(returnCode) } } @@ -2248,6 +2259,58 @@ ClassMethod RunGitCommandWithInput(command As %String, inFile As %String = "", O quit returnCode } +/// Translates a <NOTOPEN> error from a failed git launch into an actionable +/// exception. The message includes the underlying OS error (from +/// $system.Process.OSError()) plus a hint targeted at the specific error code. +/// Non-<NOTOPEN> errors are returned unchanged. +ClassMethod GitLaunchError(e As %Exception.AbstractException) As %Exception.AbstractException +{ + if (e.Name '= "") { + quit e + } + // Read the OS error immediately: it is a lingering value, but the failed launch + // that produced this has just refreshed it to the real cause. + // On Windows, OSError() has a trailing newline; strip trailing control chars so + // it doesn't break the message onto an extra line. + set os = $zstrip($system.Process.OSError(), ">C") + set errno = $piece($piece(os, "<", 2), ">", 1) + set prefix = "git could not be launched"_$select(os '= "": " ("_os_")", 1: "")_"."_$c(13,10) + if (errno = 2) { + quit ##class(%Exception.General).%New("", , , prefix_..GitNotFoundHint()) + } elseif (errno = 13) { + quit ##class(%Exception.General).%New("", , , prefix_..GitNotExecutableHint()) + } + quit ##class(%Exception.General).%New("", , , prefix_..GitLaunchGenericHint()) +} + +/// Translates a failed git launch on Linux, where the /SHELL retry absorbs the +/// failure into a shell exit code (127 = command not found, 126 = found but not +/// executable) instead of throwing <NOTOPEN>, into the same actionable exception +/// that GitLaunchError produces for the <NOTOPEN> case. +ClassMethod GitLaunchFailedError(returnCode As %Integer) As %Exception.AbstractException +{ + set prefix = "git could not be launched (shell exit "_returnCode_")."_$c(13,10) + if (returnCode = 127) { + quit ##class(%Exception.General).%New("", , , prefix_..GitNotFoundHint()) + } + quit ##class(%Exception.General).%New("", , , prefix_..GitNotExecutableHint()) +} + +ClassMethod GitNotFoundHint() As %String [ CodeMode = expression ] +{ +"git was not found. Make sure git is installed and on the IRIS user's PATH, or set an absolute path to the git executable on the Settings page." +} + +ClassMethod GitNotExecutableHint() As %String [ CodeMode = expression ] +{ +"git could not be executed. Check the permissions on the git executable"_$select($$$isWINDOWS: ", and ensure the IRIS user has the ""Replace a process level token"" privilege.", 1: ".") +} + +ClassMethod GitLaunchGenericHint() As %String [ CodeMode = expression ] +{ +"Check that git is installed, on the IRIS user's PATH (or set an absolute path on the Settings page), and that the IRIS user has permission to run it." +} + ClassMethod SyncIrisWithRepoThroughCommand(ByRef outStream) As %Status { set deletedFiles = "" diff --git a/csp/gitprojectsettings.csp b/csp/gitprojectsettings.csp index 1f51ab2a..3ba4d29b 100644 --- a/csp/gitprojectsettings.csp +++ b/csp/gitprojectsettings.csp @@ -213,7 +213,7 @@ body { } } catch err { do err.Log() - &html<
An error occurred and has been logged to the application error log.
> + &html<
An error occurred and has been logged to the application error log: #(..EscapeHTML(err.DisplayString()))#
> }
diff --git a/docs/superpowers/plans/2026-07-21-notopen-error-reporting.md b/docs/superpowers/plans/2026-07-21-notopen-error-reporting.md new file mode 100644 index 00000000..42e77e91 --- /dev/null +++ b/docs/superpowers/plans/2026-07-21-notopen-error-reporting.md @@ -0,0 +1,543 @@ +# Improve `` Error Reporting Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the raw, uncaught `` error from failed git launches with an actionable message, add on-demand active diagnostics at Configure/Settings, and document the error in the README. + +**Architecture:** All runtime logic lives in `SourceControl.Git.Utils`. `RunGitCommandWithInput` catches `` from the `$zf(-100)` call and re-throws a static multi-cause message via a new `GitLaunchError` helper. A separate `DiagnoseGitLaunch` method actively probes for the specific cause and is invoked at the end of `SourceControl.Git.API:Configure` and on load of `csp/gitprojectsettings.csp`. Docs live in `README.md`. + +**Tech Stack:** InterSystems ObjectScript, IRIS `%UnitTest.TestCase`, CSP. + +## Global Constraints + +- Language: InterSystems ObjectScript. Follow existing conventions in `SourceControl.Git.Utils` (lowercase `set`/`quit`/`do`, `##class(...)`, `$$$` macros). +- Only `` errors are translated. Any other exception must re-throw unchanged so unrelated failures are not masked. +- The runtime failure-time message must NOT contain a docs link / URL. +- The three known causes, referenced verbatim where a message lists them: + 1. git is not on the IRIS user's PATH (set an absolute path to the git executable in Configure / the Settings page). + 2. The IRIS user lacks permission on the repository directory. + 3. Windows: the IRIS user lacks the "Replace a process level token" privilege. +- The InterSystems `$zf(-100)` error-handling link appears in `README.md` ONLY: + `https://docs.intersystems.com/irislatest/csp/docbook/DocBook.UI.Page.cls?KEY=RCOS_fzf-100#RCOS_fzf-100_error_handling` +- Test classes extend `UnitTest.SourceControl.Git.AbstractTest` and live under `test/UnitTest/SourceControl/Git/`. +- Do not commit to `main`. Work is on branch `diagnose-notopen-errors`. + +## File Structure + +- Modify: `cls/SourceControl/Git/Utils.cls` + - Add `GitLaunchError` classmethod (builds the actionable exception). + - Add `DiagnoseGitLaunch` classmethod (active probes; returns healthy boolean + message). + - Wrap the `$zf(-100)` block in `RunGitCommandWithInput` to translate ``. +- Modify: `cls/SourceControl/Git/API.cls` + - Call `DiagnoseGitLaunch` at the end of `Configure` and print the message if unhealthy. +- Modify: `csp/gitprojectsettings.csp` + - Call `DiagnoseGitLaunch` on page load and render the message near the git-path field if unhealthy. +- Modify: `README.md` + - Add a Troubleshooting section for ``. +- Create: `test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls` + - Unit tests for `GitLaunchError` and `DiagnoseGitLaunch`. + +--- + +### Task 1: `GitLaunchError` helper — translate `` to an actionable exception + +**Files:** +- Modify: `cls/SourceControl/Git/Utils.cls` (add classmethod near `RunGitCommandWithInput`, after line 2249) +- Test: `test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls` (create) + +**Interfaces:** +- Consumes: nothing from other tasks. +- Produces: + - `ClassMethod GitLaunchError(e As %Exception.AbstractException) As %Exception.AbstractException` — returns a `%Exception.General` whose message lists the three causes. If `e.Name '= ""`, returns `e` unchanged. + - Message text (used by Task 3 assertions) contains the substrings `"git could not be launched"`, `"PATH"`, `"permission"`, and `"Replace a process level token"`. It contains NO `"http"` substring. + +- [ ] **Step 1: Write the failing test** + +Create `test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls`: + +```objectscript +Class UnitTest.SourceControl.Git.Utils.GitLaunchError Extends UnitTest.SourceControl.Git.AbstractTest +{ + +Method TestTranslatesNotOpen() +{ + set src = ##class(%Exception.General).%New("", "", "", "") + set src.Name = "" + set translated = ##class(SourceControl.Git.Utils).GitLaunchError(src) + set msg = translated.DisplayString() + do $$$AssertTrue(msg [ "git could not be launched", "message explains git launch failed") + do $$$AssertTrue(msg [ "PATH", "message mentions PATH cause") + do $$$AssertTrue(msg [ "permission", "message mentions permission cause") + do $$$AssertTrue(msg [ "Replace a process level token", "message mentions Windows token privilege") + do $$$AssertTrue('(msg [ "http"), "runtime message contains no docs link") +} + +Method TestPassesThroughOtherErrors() +{ + set src = ##class(%Exception.General).%New("", "", "", "") + set src.Name = "" + set translated = ##class(SourceControl.Git.Utils).GitLaunchError(src) + do $$$AssertEquals(translated.Name, "", "non-NOTOPEN error passes through unchanged") +} + +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run (in the IRIS container terminal for the namespace under test): +``` +do ##class(%UnitTest.Manager).RunTest("UnitTest.SourceControl.Git.Utils.GitLaunchError") +``` +Expected: FAIL — `GitLaunchError` does not exist (``). + +- [ ] **Step 3: Write minimal implementation** + +In `cls/SourceControl/Git/Utils.cls`, add after `RunGitCommandWithInput` (after line 2249): + +```objectscript +/// Translates a <NOTOPEN> error from a failed git launch into an actionable +/// exception listing the known causes. Non-<NOTOPEN> errors are returned unchanged. +ClassMethod GitLaunchError(e As %Exception.AbstractException) As %Exception.AbstractException +{ + if (e.Name '= "") { + quit e + } + set msg = ": git could not be launched. Common causes:"_$c(13,10) + set msg = msg_" 1. git is not on the IRIS user's PATH (set an absolute path to the git executable in Configure / the Settings page)."_$c(13,10) + set msg = msg_" 2. The IRIS user lacks permission on the repository directory."_$c(13,10) + set msg = msg_" 3. Windows: the IRIS user lacks the ""Replace a process level token"" privilege." + quit ##class(%Exception.General).%New("", , , msg) +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: +``` +do ##class(%UnitTest.Manager).RunTest("UnitTest.SourceControl.Git.Utils.GitLaunchError") +``` +Expected: PASS for `TestTranslatesNotOpen` and `TestPassesThroughOtherErrors`. + +- [ ] **Step 5: Commit** + +```bash +git add cls/SourceControl/Git/Utils.cls test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls +git commit -m "Add GitLaunchError helper to translate git launch failures (#462)" +``` + +--- + +### Task 2: Translate `` at the `$zf(-100)` call site + +**Files:** +- Modify: `cls/SourceControl/Git/Utils.cls:2172-2184` (the `try/catch` around `$zf(-100)` in `RunGitCommandWithInput`) + +**Interfaces:** +- Consumes: `GitLaunchError` from Task 1. +- Produces: `RunGitCommandWithInput` throws the translated exception on `` instead of letting it escape raw. No signature change. + +- [ ] **Step 1: Review the current block** + +Current code at `cls/SourceControl/Git/Utils.cls:2172-2184`: + +```objectscript + try { + // Inject instance manager directory as global git config home directory + // On Linux, this avoids trying to use /root/.config/git/attributes for global git config + set env("XDG_CONFIG_HOME") = ##class(%File).ManagerDirectory() + set returnCode = $zf(-100,"/ENV=env... "_baseArgs,gitCommand,newArgs...) + } catch e { + if $$$isWINDOWS { + set returnCode = $zf(-100,baseArgs,gitCommand,newArgs...) + } else { + // If can't inject XDG_CONFIG_HOME (older IRIS version), need /SHELL on Linux to avoid permissions errors trying to use root's config + set returnCode = $zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...) + } + } +``` + +Note: the outer `catch e` handles the *expected* failure of the `/ENV` variant on older IRIS, then retries. That retry is currently unguarded. + +- [ ] **Step 2: Rewrite the block to guard the retry and translate ``** + +Replace lines 2172-2184 with: + +```objectscript + try { + // Inject instance manager directory as global git config home directory + // On Linux, this avoids trying to use /root/.config/git/attributes for global git config + set env("XDG_CONFIG_HOME") = ##class(%File).ManagerDirectory() + set returnCode = $zf(-100,"/ENV=env... "_baseArgs,gitCommand,newArgs...) + } catch e { + // /ENV injection failed (older IRIS); retry without it. + try { + if $$$isWINDOWS { + set returnCode = $zf(-100,baseArgs,gitCommand,newArgs...) + } else { + // If can't inject XDG_CONFIG_HOME (older IRIS version), need /SHELL on Linux to avoid permissions errors trying to use root's config + set returnCode = $zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...) + } + } catch retryErr { + throw ..GitLaunchError(retryErr) + } + } +``` + +Note: the first `$zf(-100)` failing throws into `catch e` and is retried (existing behavior — do not translate it there, since `/ENV` failure is expected on older IRIS). Only the retry's failure is translated. This matches the observed stack in the issue, where the failure surfaced from the retry line. + +- [ ] **Step 3: Compile and smoke-test a normal git command** + +Compile `SourceControl.Git.Utils`. In a namespace with git correctly configured, run: +``` +do ##class(SourceControl.Git.Utils).RunGitCommand("--version",.err,.out) write out.ReadLine(),! +``` +Expected: prints `git version ...` (translation path not triggered; normal operation unaffected). + +- [ ] **Step 4: Verify translation with a bad git path** + +Set an invalid absolute git path, then run a real command: +``` +set $namespace = $namespace +set ^["^^%SYS"]... // (skip) — instead use the storage node: +do ##class(SourceControl.Git.Utils).RunGitCommand("status",.err,.out) +``` +If a live `` cannot be reliably reproduced in the dev environment, this step is covered by Task 3's `DiagnoseGitLaunch` test; note that here and move on. + +- [ ] **Step 5: Commit** + +```bash +git add cls/SourceControl/Git/Utils.cls +git commit -m "Translate from git launch retry into actionable error (#462)" +``` + +--- + +### Task 3: `DiagnoseGitLaunch` — active probes + +**Files:** +- Modify: `cls/SourceControl/Git/Utils.cls` (add classmethod after `GitLaunchError`) +- Test: `test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls` (add methods) + +**Interfaces:** +- Consumes: `GitBinPath(.isDefault)` ([Utils.cls:118](../../../cls/SourceControl/Git/Utils.cls#L118)), `TempFolder()` ([Utils.cls:41](../../../cls/SourceControl/Git/Utils.cls#L41)), `RunGitCommand`. +- Produces: + - `ClassMethod DiagnoseGitLaunch(Output userMessage As %String) As %Boolean` — returns `1` and sets `userMessage=""` when git launches successfully; returns `0` with a diagnostic `userMessage` when it fails. + +- [ ] **Step 1: Write the failing test** + +Add to `test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls`: + +```objectscript +Method TestDiagnoseHealthyWhenGitWorks() +{ + // AbstractTest configures a valid environment; git --version should launch. + set healthy = ##class(SourceControl.Git.Utils).DiagnoseGitLaunch(.msg) + do $$$AssertTrue(healthy, "git launches in the test environment") + do $$$AssertEquals(msg, "", "healthy diagnosis returns empty message") +} + +Method TestDiagnoseReportsBadGitPath() +{ + // Point the git bin path at a non-existent absolute path. + set storage = ##class(SourceControl.Git.Utils).%SYSNamespaceStorage() + set saved = $get(@storage@("%gitBinPath")) + try { + set @storage@("%gitBinPath") = "/nonexistent/path/to/git-binary-xyz" + set healthy = ##class(SourceControl.Git.Utils).DiagnoseGitLaunch(.msg) + do $$$AssertNotTrue(healthy, "diagnosis reports unhealthy for bad git path") + do $$$AssertTrue(msg [ "git", "message references git") + } catch ex { + do $$$AssertStatusOK(ex.AsStatus()) + } + if (saved = "") { + kill @storage@("%gitBinPath") + } else { + set @storage@("%gitBinPath") = saved + } +} +``` + +Note: confirm the storage accessor name — `%SYSNamespaceStorage()` is referenced at `cls/SourceControl/Git/Utils.cls:123`. If the accessor differs, use the exact name found there. + +- [ ] **Step 2: Run test to verify it fails** + +Run: +``` +do ##class(%UnitTest.Manager).RunTest("UnitTest.SourceControl.Git.Utils.GitLaunchError") +``` +Expected: FAIL — `DiagnoseGitLaunch` does not exist. + +- [ ] **Step 3: Write minimal implementation** + +In `cls/SourceControl/Git/Utils.cls`, add after `GitLaunchError`: + +```objectscript +/// Actively probes why git cannot be launched. Returns 1 (healthy) with an empty +/// userMessage when git launches; returns 0 with a diagnostic message otherwise. +ClassMethod DiagnoseGitLaunch(Output userMessage As %String) As %Boolean +{ + set userMessage = "" + // Try a real launch first; if it works, nothing else to report. + try { + do ..RunGitCommand("--version", .err, .out) + if $isobject($get(out)) && (out.ReadLine() [ "git version") { + quit 1 + } + } catch e { + if (e.Name '= "") { + // Unrelated failure; surface its text but don't run cause probes. + set userMessage = $system.Status.GetErrorText(e.AsStatus()) + quit 0 + } + } + + // Launch failed with : probe the known causes. + set userMessage = "git could not be launched. Diagnosis:"_$c(13,10) + set binPath = $extract(..GitBinPath(.isDefault), 2, *-1) + if isDefault { + set userMessage = userMessage_" - No absolute git path is configured; git is being resolved via the IRIS user's PATH. If git is not on that PATH, set an absolute path in the Settings page."_$c(13,10) + } elseif '##class(%File).Exists(binPath) { + set userMessage = userMessage_" - The configured git executable was not found at: "_binPath_$c(13,10) + } else { + set userMessage = userMessage_" - The configured git executable exists at "_binPath_" but could not be launched."_$c(13,10) + } + + set repoDir = ..TempFolder() + if '##class(%File).DirectoryExists(repoDir) { + set userMessage = userMessage_" - The repository directory does not exist: "_repoDir_$c(13,10) + } elseif '..DirectoryWritable(repoDir) { + set userMessage = userMessage_" - The IRIS user cannot write to the repository directory: "_repoDir_" (check ownership / permissions)."_$c(13,10) + } + + if $$$isWINDOWS { + set userMessage = userMessage_" - On Windows, ensure the IRIS user has the ""Replace a process level token"" privilege."_$c(13,10) + } + quit 0 +} + +/// Returns 1 if the IRIS user can create a file in the given directory. +ClassMethod DirectoryWritable(dir As %String) As %Boolean +{ + set probe = ##class(%File).NormalizeFilename(dir_"/.eg-writeprobe") + set stream = ##class(%Stream.FileCharacter).%New() + set stream.Filename = probe + do stream.Write("probe") + set writable = $$$ISOK(stream.%Save()) + do ##class(%File).Delete(probe) + quit writable +} +``` + +Note: verify `..#Slash` vs `/` for `probe`; `%File.NormalizeFilename` handles both separators, so `/` is safe here. + +- [ ] **Step 4: Run test to verify it passes** + +Run: +``` +do ##class(%UnitTest.Manager).RunTest("UnitTest.SourceControl.Git.Utils.GitLaunchError") +``` +Expected: PASS for all four methods. + +- [ ] **Step 5: Commit** + +```bash +git add cls/SourceControl/Git/Utils.cls test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls +git commit -m "Add DiagnoseGitLaunch active probe for git launch failures (#462)" +``` + +--- + +### Task 4: Invoke diagnostics at end of `Configure` + +**Files:** +- Modify: `cls/SourceControl/Git/API.cls:25-30` (inside `Configure`, after `Settings.Configure()` succeeds) + +**Interfaces:** +- Consumes: `DiagnoseGitLaunch` from Task 3. +- Produces: end-user terminal output when git launch is unhealthy. + +- [ ] **Step 1: Add the diagnostic call** + +In `cls/SourceControl/Git/API.cls`, after the `tcommit` at line 30 (still inside the `try`), before the closing brace of the `try`, add: + +```objectscript + if '##class(SourceControl.Git.Utils).DiagnoseGitLaunch(.gitDiagMsg) { + write !,gitDiagMsg + } +``` + +The surrounding block becomes: + +```objectscript + set good = ##class(SourceControl.Git.Settings).Configure() + if 'good { + write !,"Cancelled." + quit + } + tcommit + if '##class(SourceControl.Git.Utils).DiagnoseGitLaunch(.gitDiagMsg) { + write !,gitDiagMsg + } +``` + +- [ ] **Step 2: Compile and smoke-test** + +Compile `SourceControl.Git.API`. In a namespace with valid git, run: +``` +do ##class(SourceControl.Git.API).Configure() +``` +(accept defaults). Expected: completes with no diagnostic message printed (git healthy). + +- [ ] **Step 3: Verify unhealthy path (optional, manual)** + +Temporarily set an invalid git path via the Settings prompt during Configure, or set the storage node as in Task 3's test, then re-run `Configure`. Expected: the diagnostic message prints after configuration. + +- [ ] **Step 4: Commit** + +```bash +git add cls/SourceControl/Git/API.cls +git commit -m "Run git launch diagnostics at end of Configure (#462)" +``` + +--- + +### Task 5: Render diagnostics on the Settings page + +**Files:** +- Modify: `csp/gitprojectsettings.csp:285-311` (the git-path fieldset / feedback area) + +**Interfaces:** +- Consumes: `DiagnoseGitLaunch` from Task 3. +- Produces: an on-page message near the git-path field when git launch is unhealthy. + +- [ ] **Step 1: Add the diagnostic render** + +In `csp/gitprojectsettings.csp`, inside the existing `` block around lines 289-310 (which already calls `GitBinExists`), after the feedback variables are computed, add a diagnosis probe and render it below the field. Insert after line 310 (before the closing ``): + +```html + set gitLaunchHealthy = ##class(SourceControl.Git.Utils).DiagnoseGitLaunch(.gitLaunchMsg) +``` + +Then, immediately after the closing `` for this block and after the input's feedback `
`, add the message rendering: + +```html + + if 'gitLaunchHealthy { + &html<
#($zconvert(gitLaunchMsg,"O","HTML"))#
> + } + +``` + +Note: `white-space: pre-line;` preserves the `$c(13,10)` line breaks from the message. `$zconvert(...,"O","HTML")` escapes the message for safe HTML output. + +- [ ] **Step 2: Load the Settings page and verify (healthy)** + +Open the Git project settings page in a namespace with valid git. Expected: no warning alert appears (git healthy). + +- [ ] **Step 3: Verify unhealthy path (manual)** + +Set an invalid absolute git path via the field and save, or set the storage node as in Task 3's test, then reload the page. Expected: a yellow warning alert with the multi-line diagnosis renders below the git-path field. + +- [ ] **Step 4: Commit** + +```bash +git add csp/gitprojectsettings.csp +git commit -m "Show git launch diagnostics on the Settings page (#462)" +``` + +--- + +### Task 6: README Troubleshooting section + +**Files:** +- Modify: `README.md` (add a `## Troubleshooting` section; the `## Support` section begins at line 183, insert before it) + +**Interfaces:** +- Consumes: nothing. +- Produces: user-facing docs. Cross-references the existing "Dubious Ownership" (line 117) and "IRIS Privileges" (line 135) subsections. + +- [ ] **Step 1: Add the section** + +In `README.md`, immediately before the `## Support` heading (line 183), add: + +```markdown +## Troubleshooting + +### `` error when running git commands + +When IRIS cannot launch the git executable, you may see an error like: + +``` +ERROR #5002: ObjectScript error: RunGitCommandWithInput+151^SourceControl.Git.Utils.1 +``` + +This means the underlying `$zf(-100)` call could not start the git process. Common causes and fixes: + +1. **git is not on the IRIS user's PATH.** git may be installed only for your own user account rather than system-wide. Set an absolute path to the git executable in the Configure prompt or on the Settings page (e.g. `C:\Program Files\Git\bin\git.exe` on Windows, `/usr/bin/git` on Unix). +2. **The IRIS user lacks permission on the repository directory.** The namespace temp (repo root) folder must be owned by / accessible to the user IRIS runs as. See [Dubious Ownership](#dubious-ownership). +3. **Windows: the IRIS user lacks the "Replace a process level token" privilege.** Grant this privilege to the account IRIS runs as. See also [IRIS Privileges](#iris-privileges). + +For more on `$zf(-100)` error handling, see the [InterSystems documentation](https://docs.intersystems.com/irislatest/csp/docbook/DocBook.UI.Page.cls?KEY=RCOS_fzf-100#RCOS_fzf-100_error_handling). + +The Configure script and the Settings page also run an automatic diagnostic that reports which of these causes is most likely. +``` + +- [ ] **Step 2: Verify anchors** + +Confirm the markdown anchors `#dubious-ownership` and `#iris-privileges` match the existing headings `#### Dubious Ownership` and `#### IRIS Privileges`. GitHub lowercases and hyphenates heading text for anchors, so these are correct. + +- [ ] **Step 3: Commit** + +```bash +git add README.md +git commit -m "Document git launch error in README Troubleshooting (#462)" +``` + +--- + +### Task 7: Update CHANGELOG + +**Files:** +- Modify: `CHANGELOG.md` (add an entry under the unreleased/next section) + +**Interfaces:** +- Consumes: nothing. +- Produces: changelog entry. + +- [ ] **Step 1: Add the entry** + +Open `CHANGELOG.md`, find the top/unreleased section, and add a bullet matching the existing format: + +```markdown +- Improved reporting of `` errors on the git executable, with an actionable message and diagnostics pointing to likely causes (#462) +``` + +Match the exact heading/format already used in the file for the current unreleased entries. + +- [ ] **Step 2: Commit** + +```bash +git add CHANGELOG.md +git commit -m "Update CHANGELOG for error reporting (#462)" +``` + +--- + +## Self-Review + +**Spec coverage:** +- Catch & translate `` → Tasks 1, 2. ✓ +- On-demand active diagnostics (end of Configure + Settings load) → Tasks 3, 4, 5. ✓ +- Troubleshooting docs in README with the InterSystems link → Task 6. ✓ +- Logging explicitly out of scope → no task. ✓ (consistent with spec) +- Runtime message has no docs link → enforced by Global Constraints and Task 1 test `TestTranslatesNotOpen` (`msg` contains no `"http"`). ✓ +- Link appears in README only → Task 6. ✓ + +**Placeholder scan:** Task 2 Step 4 notes a live `` may be hard to reproduce and defers to Task 3's test — this is a conditional verification note, not a code placeholder; acceptable. No "TBD"/"TODO" in delivered code. + +**Type consistency:** +- `GitLaunchError(e)` — defined Task 1, consumed Task 2. ✓ +- `DiagnoseGitLaunch(.userMessage)` returning `%Boolean` — defined Task 3, consumed Tasks 4, 5. ✓ +- `DirectoryWritable(dir)` helper — defined and used within Task 3. ✓ +- Storage accessor `%SYSNamespaceStorage()` referenced in Task 3 test — flagged to verify against `Utils.cls:123`. ✓ diff --git a/docs/superpowers/specs/2026-07-21-notopen-error-reporting-design.md b/docs/superpowers/specs/2026-07-21-notopen-error-reporting-design.md new file mode 100644 index 00000000..96e77143 --- /dev/null +++ b/docs/superpowers/specs/2026-07-21-notopen-error-reporting-design.md @@ -0,0 +1,233 @@ +# Design: Improve `` error reporting (issue #462) + +## Revision (2026-07-21): use `$system.Process.OSError()` instead of active probes + +After the initial implementation (Tasks 1-7, all committed), empirical testing in the +container changed the design. `$system.Process.OSError()` returns the actual OS-level +error behind a failed `$zf(-100)` launch, which is more accurate and self-maintaining +than a static list of causes or active probes. + +Confirmed by reproduction in the Linux container: + +| Scenario | ``? | `OSError()` | +|---|---|---| +| git executable not found (not on PATH / bad absolute path) | Yes | `<2> No such file or directory` (ENOENT) | +| git exists but is not executable | Yes | `<13> Permission denied` (EACCES) | +| git launches but the repo dir is inaccessible | **No** | git returns code 128 with a clear stderr `fatal: cannot change to '...'` — already handled by the normal stderr flow | + +Key facts established: + +- `OSError()` is a *lingering* "last OS error": it does NOT clear after a subsequent + *successful* command. BUT it *is* correctly refreshed whenever a *new* launch failure + occurs (seeded errno 13, then a missing-binary launch correctly reported errno 2). So + it is reliable **when read in the catch block immediately after the failed launch**, + which is exactly where `GitLaunchError` runs. It must NOT be read unconditionally on a + success path. +- The "repo directory permissions" cause from the original issue thread does not surface + as `` on Linux — git launches and returns code 128 with a descriptive stderr + message. The `` path is specifically about launching the git process itself. +- `OSError()` is platform-specific; errno 2/13 are POSIX. Windows returns different + codes, so the design maps the known POSIX codes to specific hints and falls back to + showing the raw `OSError()` text (which is human-readable on all platforms) for any + other code. + +### Revised architecture + +1. **Failure-time translation** (`GitLaunchError`) — read `$system.Process.OSError()` in + the catch and build a message combining the actual OS error with a targeted hint: + - errno 2 → git not found; set an absolute path in Settings / add git to PATH. + - errno 13 → git could not be executed; check the executable's permissions and (on + Windows) the "Replace a process level token" privilege. + - any other → show the raw `OSError()` text plus a generic troubleshooting pointer. + Still no docs link in the runtime message. +2. **Active diagnostics REMOVED.** `DiagnoseGitLaunch` and `DirectoryWritable`, and their + call sites in `Configure` and `csp/gitprojectsettings.csp`, are removed. `OSError()` + already pinpoints the launch failure, so the probes add complexity without value. +3. **Troubleshooting docs** — unchanged; the README section stays (with the InterSystems + `$zf(-100)` link). + +The sections below describe the ORIGINAL design (Tasks 1-7). Where they conflict with +this revision, this revision governs. + +### Follow-up (2026-07-21): the retry's `` translation was Windows-only + +Manual testing in the Linux container found that `GitLaunchError`, as written, never +fired on Linux. The retry branch in `RunGitCommandWithInput` is platform-specific: + +- **Windows retry:** `$zf(-100,baseArgs,gitCommand,newArgs...)` — a failed launch + throws ``, so `GitLaunchError` runs as designed. +- **Linux retry:** `$zf(-100,"/SHELL "_baseArgs,gitCommand,newArgs...)` — `/SHELL` + shells out to launch git, so a launch failure is absorbed into a normal shell exit + code instead of throwing: **127** (command not found) or **126** (found, not + executable). `RunGitCommandWithInput` simply returns 127/126 with no exception and no + actionable message. + +Confirmed empirically (missing git path, then a non-executable git path, both run +through the real Linux `/SHELL` retry): + +| Configured git path | Windows retry (`$zf` no `/SHELL`) | Linux retry (`/SHELL`) | +|---|---|---| +| missing | throws ``, OSError `<2>` | returns 127, no throw, OSError cleared | +| exists, not executable | throws ``, OSError `<13>` | returns 126, no throw | + +git itself never uses exit codes 126/127 for its own errors (confirmed: an unknown git +subcommand exits 1) — those codes are shell-reserved for "command not found" / +"found but not executable" when the shell can't launch the target at all. So detecting +127/126 specifically after the Linux retry is safe and cannot misfire on a real git +failure. + +Fix: after the retry's `try/catch` in `RunGitCommandWithInput`, on non-Windows, check +`returnCode` for 127/126 and throw a new `GitLaunchFailedError(returnCode)` that shares +its hint text with `GitLaunchError` (extracted into `GitNotFoundHint`, +`GitNotExecutableHint`, `GitLaunchGenericHint`). This makes the actionable-message +behavior consistent across both platforms. + +## Problem + +When git cannot be launched via `$zf(-100)`, IRIS raises a raw `` error that +propagates uncaught out of `SourceControl.Git.Utils:RunGitCommandWithInput`, aborting +the whole flow (e.g. the `Configure` / `Clone` setup script) with a stack like: + +``` +ERROR #5002: ObjectScript error: RunGitCommandWithInput+151^SourceControl.Git.Utils.1 +``` + +The message tells the user nothing about the actual cause or the fix. + +Known causes (from the issue thread): + +1. The git executable is not on the IRIS operating user's PATH, and no absolute path + to the git executable was provided through the Configure prompt. +2. Permissions issues where the IRIS operating user does not own / cannot access the + Git repo root (namespace temp) directory. +3. On Windows, the IRIS operating user lacks the "Replace a process level token" + privilege. + +## Scope + +In scope: + +- Catch the `` at failure time and re-throw an actionable message. +- On-demand active diagnostics at two explicit checkpoints (end of `Configure`, + Settings page load) that probe for the specific cause. +- A Troubleshooting section in `README.md`. + +Out of scope: + +- Logging these launch failures to `SourceControl_Git.Log`. The `$zf(-100)` failure + occurs before the log record is written, and logging was explicitly excluded. + +## Architecture + +Three coordinated pieces. No new classes; all runtime changes live in +`SourceControl.Git.Utils`, `SourceControl.Git.API`, and `csp/gitprojectsettings.csp`. + +1. **Failure-time translation** (inline, cheap) — wrap the `$zf(-100)` calls in + `RunGitCommandWithInput` so a `` is caught and re-thrown as a clear, + static, multi-cause message. +2. **On-demand active diagnostics** (`DiagnoseGitLaunch`) — a heavier probe method run + only at two explicit checkpoints. It actually tests each known cause and reports + which failed. +3. **Troubleshooting docs** — a new Troubleshooting section in `README.md`. + +## Piece 1 — Failure-time translation (inline) + +Location: `RunGitCommandWithInput`, currently +[Utils.cls:2172-2184](../../../cls/SourceControl/Git/Utils.cls#L2172-L2184). + +The current `try/catch` catches an error from the first `$zf(-100)` (the `/ENV` +variant) and retries without `/ENV`. That inner retry is itself unguarded — a +`` there propagates raw. + +Change: keep the existing `/ENV`-fallback logic, but wrap the whole block so any +`` (from either attempt) is caught and re-thrown as an actionable exception, +built by a new helper: + +```objectscript +ClassMethod GitLaunchError(e As %Exception.AbstractException) As %Exception.AbstractException +``` + +- Builds a static message listing the three known causes. **No docs link / URL in the + runtime message.** +- Returns an exception carrying that text; `RunGitCommandWithInput` throws it. +- Only `` (`e.Name = ""`) is translated. Any other error re-throws + unchanged, so unrelated failures are not masked. + +Interaction notes: + +- Because the failure happens before the log write at + [Utils.cls:2247](../../../cls/SourceControl/Git/Utils.cls#L2247), these failures still + will not reach `SourceControl_Git.Log` — consistent with the out-of-scope decision. +- `GitBinExists` calls `RunGitCommand("--version")` inside its own `try/catch` + ([Utils.cls:108-113](../../../cls/SourceControl/Git/Utils.cls#L108-L113)) and swallows + the error to return `version = ""`. The new throw does not disrupt that path since it + is already caught there. + +Runtime message shape (no link): + +``` +: git could not be launched. Common causes: + 1. git is not on the IRIS user's PATH (set an absolute path to the git + executable in Configure / the Settings page). + 2. The IRIS user lacks permission on the repository directory. + 3. Windows: the IRIS user lacks the "Replace a process level token" privilege. +``` + +## Piece 2 — On-demand active diagnostics + +New method: + +```objectscript +ClassMethod DiagnoseGitLaunch(Output userMessage As %String) As %Boolean +``` + +Behavior: + +- Attempts a real git invocation (`git --version` through the same `$zf(-100)` path). +- Returns `1` (healthy) with an empty `userMessage` on success. +- On ``, runs the active probes and returns `0` with a `userMessage` + reporting which check(s) failed: + - **git resolvable** — is `GitBinPath()` the default `"git"` (PATH-dependent) or an + absolute path? If absolute, does the file exist + (`##class(%File).Exists`)? If default, note the PATH dependency. + - **repo dir permissions** — does `TempFolder()` exist and is it writable by the + IRIS user (attempt a temp-file write / ownership check)? + - **Windows token privilege** — on Windows, surface the "Replace a process level + token" privilege as a likely cause (hard to probe directly, so reported as a + likely cause rather than a definitive check). + +Checkpoints (same probe set at both): + +1. **End of `Configure()`** ([API.cls:5-38](../../../cls/SourceControl/Git/API.cls#L5-L38)) + — after settings are saved, run the diagnostic and `write` the message to the + terminal if unhealthy. +2. **Settings page load** + ([gitprojectsettings.csp:289-310](../../../csp/gitprojectsettings.csp#L289-L310)) — + render the diagnostic message near the git-path field when unhealthy. + +## Piece 3 — Troubleshooting docs (README.md) + +Add a "Troubleshooting" section to `README.md`. It documents the `` error: + +- What the error looks like (the `...RunGitCommandWithInput` stack). +- The three known causes and their fixes: + 1. git not on the IRIS user's PATH → set an absolute path to the git executable in + Configure / the Settings page. + 2. IRIS user lacks ownership / permissions on the repo root directory → fix + ownership / permissions (cross-reference the existing "Dubious Ownership" section). + 3. Windows: grant the IRIS user the "Replace a process level token" privilege + (cross-reference the existing "IRIS Privileges" section). +- Include the InterSystems `$zf(-100)` error-handling link **in the docs only** (not in + the runtime message): + https://docs.intersystems.com/irislatest/csp/docbook/DocBook.UI.Page.cls?KEY=RCOS_fzf-100#RCOS_fzf-100_error_handling + +## Testing + +- Unit-level: verify `GitLaunchError` translates a `` exception into a + message containing all three causes, and passes non-`` errors through + unchanged. +- `DiagnoseGitLaunch`: verify it returns healthy when git launches, and returns an + unhealthy message identifying the git-path probe result when the configured path is + invalid (e.g. point `%gitBinPath` at a non-existent absolute path). +- Manual: confirm the message renders on the Settings page and prints at the end of + `Configure` when git is misconfigured. diff --git a/test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls b/test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls new file mode 100644 index 00000000..20c50ed7 --- /dev/null +++ b/test/UnitTest/SourceControl/Git/Utils/GitLaunchError.cls @@ -0,0 +1,125 @@ +Class UnitTest.SourceControl.Git.Utils.GitLaunchError Extends UnitTest.SourceControl.Git.AbstractTest +{ + +/// Triggers a real by launching a nonexistent executable, passes the caught +/// exception through GitLaunchError, and verifies the message reports the actual OS +/// error (ENOENT) plus the "not found" hint, with no docs link. +Method TestTranslatesNotOpenForMissingBinary() +{ + set outLog = ##class(%Library.File).TempFilename() + set errLog = ##class(%Library.File).TempFilename() + set baseArgs = "/STDOUT="_""""_outLog_""""_" /STDERR="_""""_errLog_"""" + set translated = "" + try { + set rc = $zf(-100, baseArgs, "/nonexistent/path/to/git-binary-xyz", "--version") + } catch e { + set translated = ##class(SourceControl.Git.Utils).GitLaunchError(e) + } + do ##class(%File).Delete(outLog) + do ##class(%File).Delete(errLog) + + do $$$AssertTrue($isobject(translated), "a was raised and translated") + if '$isobject(translated) { + quit + } + set msg = translated.DisplayString() + do $$$AssertEquals(translated.Name, "", "translated exception keeps the name") + do $$$AssertTrue(msg [ "git could not be launched", "message explains git launch failed") + do $$$AssertTrue(msg [ "git was not found", "errno 2 maps to the not-found hint") + // The OS error text is embedded; on POSIX this is "No such file or directory". + do $$$AssertTrue(msg [ "No such file or directory", "message includes the underlying OS error text") + do $$$AssertTrue('(msg [ "http"), "runtime message contains no docs link") +} + +/// A whose OS error code is neither ENOENT nor EACCES falls back to the +/// generic hint. OSError() is a lingering value, so this is verified indirectly: any +/// translation must still produce an actionable, link-free message. +Method TestTranslatesNotOpenIsActionable() +{ + set src = ##class(%Exception.General).%New("", "", "", "") + set translated = ##class(SourceControl.Git.Utils).GitLaunchError(src) + set msg = translated.DisplayString() + do $$$AssertEquals(translated.Name, "", "translated exception keeps the name") + do $$$AssertTrue(msg [ "git could not be launched", "message explains git launch failed") + do $$$AssertTrue(msg [ "git", "message references git and how to fix it") + do $$$AssertTrue('(msg [ "http"), "runtime message contains no docs link") +} + +Method TestPassesThroughOtherErrors() +{ + set src = ##class(%Exception.General).%New("", "", "", "") + set src.Name = "" + set translated = ##class(SourceControl.Git.Utils).GitLaunchError(src) + do $$$AssertEquals(translated.Name, "", "non-NOTOPEN error passes through unchanged") +} + +/// On Linux, a launch failure inside the /SHELL retry does NOT throw -- it +/// returns shell exit code 127 (command not found). RunGitCommand must still raise an +/// actionable error rather than silently returning 127. +Method TestRunGitCommandThrowsForMissingConfiguredPath() +{ + set storage = ##class(SourceControl.Git.Utils).%SYSNamespaceStorage() + set saved = $get(@storage@("%gitBinPath")) + set threw = 0 + try { + set @storage@("%gitBinPath") = "/nonexistent/path/to/git-binary-xyz" + kill ^||GitVersion + try { + do ##class(SourceControl.Git.Utils).RunGitCommand("status", .err, .out) + } catch e { + set threw = 1 + do $$$AssertEquals(e.Name, "", "missing configured git path raises ") + set msg = e.DisplayString() + do $$$AssertTrue(msg [ "git was not found", "message identifies the missing git executable") + } + do $$$AssertTrue(threw, "RunGitCommand raised an error instead of returning a bare shell exit code") + } catch ex { + do $$$AssertStatusOK(ex.AsStatus()) + } + if (saved = "") { + kill @storage@("%gitBinPath") + } else { + set @storage@("%gitBinPath") = saved + } + kill ^||GitVersion +} + +/// Same shell-retry gap, for a configured path that exists but is not executable +/// (shell exit 126). +Method TestRunGitCommandThrowsForNonExecutableConfiguredPath() +{ + set fake = "/tmp/eg-test-fakegit-"_$job + set stream = ##class(%Stream.FileCharacter).%New() + set stream.Filename = fake + do stream.Write("#!/bin/sh"_$c(10)) + do stream.%Save() + do $zf(-100, "/SHELL", "chmod", "600", fake) + + set storage = ##class(SourceControl.Git.Utils).%SYSNamespaceStorage() + set saved = $get(@storage@("%gitBinPath")) + set threw = 0 + try { + set @storage@("%gitBinPath") = fake + kill ^||GitVersion + try { + do ##class(SourceControl.Git.Utils).RunGitCommand("status", .err, .out) + } catch e { + set threw = 1 + do $$$AssertEquals(e.Name, "", "non-executable configured git path raises ") + set msg = e.DisplayString() + do $$$AssertTrue(msg [ "git could not be executed", "message identifies the non-executable git path") + } + do $$$AssertTrue(threw, "RunGitCommand raised an error instead of returning a bare shell exit code") + } catch ex { + do $$$AssertStatusOK(ex.AsStatus()) + } + if (saved = "") { + kill @storage@("%gitBinPath") + } else { + set @storage@("%gitBinPath") = saved + } + kill ^||GitVersion + do ##class(%File).Delete(fake) +} + +}