Report failures and warnings to the server, tagged with which they are - #152
Merged
Conversation
handleError() printed a banner and exited, making no request at all, so a real imaging failure -- bad image, mount failure, partition error, any of the handleError call sites across fog.download, fog.upload and fog.mount -- was invisible to the FOG server. HOST_IMAGE_FAIL could not fire for it and the notification plugins registered for that event had never run on any server. The only failure FOG ever heard about was a storage node problem, through fog.checkmount -> blame.php, and that re-queues rather than fails. It now posts to service/taskerror.php, added in fogproject#1206. Best effort, and deliberately so: this must not change anything the person standing in front of the machine sees or waits for. Time bounded at five seconds, output discarded, exit status ignored, and skipped entirely when $web is unset. A server that predates the endpoint answers 404 and nothing here notices. printf %b first, so the "\n" the callers embed in their message becomes a real newline rather than two literal characters; the server flattens control characters back to spaces at its end. Co-authored-by: Claude <noreply@anthropic.com>
mastacontrola
added a commit
to FOGProject/fogproject
that referenced
this pull request
Aug 19, 2026
The server half of #1206. The FOS half is [FOGProject/fos#152](FOGProject/fos#152) — this one is useful on its own (an older FOS just never calls it), the FOS one is not useful without this. ## The gap `handleError()` in FOS prints a banner to the console and `exit 1`s. It makes **no request to the server**, so a real imaging failure — bad image, mount failure, partition error, any of the ~40 `handleError` call sites across `fog.download`, `fog.upload` and `fog.mount` — has always been invisible to FOG. The only failure the server ever heard about was a storage node problem, through `fog.checkmount` → `blame.php`, and that re-queues rather than fails. #1205 wired `HOST_IMAGE_FAIL` to `TaskQueue::checkout()`'s catch, which covers "imaging ran and FOG then failed to *record* it". This covers the failure people actually mean. `service/taskerror.php` takes the host identity FOS already sends everywhere else, plus the error text and the script that raised it, and fires `HOST_IMAGE_FAIL` with the payload #1205 defined — the same keys from either path, so a listener behaves identically whichever failure reached it. ## Deliberately narrow, in three ways **It does not change the task's state.** `taskStates` has no Failed — the five are Queued, Checked In, In-Progress, Complete, Cancelled. Reusing Cancelled loses the difference between "an admin stopped this" and "this broke"; adding a sixth means every place that enumerates states has to learn about it or a failed task becomes invisible there. That is a decision with UI and API consequences, it stays on #1206, and making the event fire does not depend on taking it. **It refuses a task that is not imaging.** `HOST_IMAGE_FAIL` is an imaging event and this endpoint is reachable from a wipe or an inventory task too. Firing it there would be the defect #1202 just removed, pointing the other way. There is no event for a non-imaging task failing; noted on the issue. **It answers `##` with 200 on every path, including every rejection.** FOS calls this on its way out and cannot act on the reply. Answering identically also means the endpoint cannot be used to ask whether a given MAC has an active imaging task. ## The text Bounded to 500 characters and flattened to one line. This is unauthenticated in the same way every other `service/*.php` is — identified by MAC, reachable by anything that can reach the web tier — and the text lands in an administrator's Slack, ntfy or pushbullet message. An embedded newline there forges what looks like a second, separate notification, which is a better lie than anything markup could manage: ``` error=Could not mount images folder (fog.mount) Args Passed: --target /images Host imaging completed successfully ``` comes out as one line, with the forged "success" visibly part of the failure text. `\p{C}` rather than a newline-only strip, because a console-facing error string can carry terminal escapes too; `preg_replace` returns null rather than throwing on invalid UTF-8, so there is a byte-wise fallback — a machine with the wrong locale must not silently report nothing. ## Not a REST route `service/*.php` is served directly and is not in `Route::defineRoutes()`, so `OpenAPI::document()` is unaffected — same as `blame.php` and the `Post_Stage` endpoints. No schema change, so no `FOG_SCHEMA` bump. No change to `Route::$validClasses`, so no FogApi sync. ## Verification Live, against this install's real database, using a throwaway host on a locally-administered MAC (`02:00:00:00:0E:11`) that matches no hardware, so nothing could ever PXE boot into the task: | case | result | |---|---| | deploy task | `FOG: imaging failed on host … (task 62): Could not mount images folder (fog.mount) Args Passed: --target /images Host imaging completed successfully` — **newlines flattened** | | capture task | notified and logged | | wipe task | refused: `Task is not an imaging task` | | unknown MAC | refused: `Invalid Host` | | 4000-char report | cut to exactly 500 | | every one of them | HTTP 200, body `##` | `HOST_IMAGE_FAIL` now appears in `notifyEvents`, which is proof `notify()` was genuinely reached and got past its guards. Fixture and shadow tree removed afterwards; host count back to 86. `tests/task-error-report.test.php`, mutation-verified — dropping the control-character strip, the length bound, the imaging gate, the uniform ack, or adding a state change each fails the suite. Full suite: `68 passed, 0 failed`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_013mJVe4CpK3rRbi9H5GubXd
Six cases over handleError(), following tests/checks/sector-size.sh: a sandbox copy of the library with its /usr/share/fog/lib path rewritten, and PATH-shadowed stubs, with curl recording its argv rather than making a request. What is pinned is not that the report arrives -- that is the server's harness, in fogproject#1207 -- but that trying to send it cannot change what the person standing at the machine sees. It goes to the right URL with mac, sysuuid and the message; it is time bounded and url-encoded; the "\n" the callers embed is expanded before sending rather than going out as two literal characters; nothing it does reaches the console; a failed report still lets handleError reach its reboot notice; and no $web means no attempt at all. Mutation-verified: dropping the $web guard, the output redirect, --max-time, the printf %b, or the whole block each fails the harness. Co-authored-by: Claude <noreply@anthropic.com>
handleError() reported to the server; handleWarning() still printed its banner, waited its minute and told nobody. A warning is the machine saying it hit something and carried on -- a disk smaller than the image it is about to take, a partition table it had to work around -- which is exactly the kind of thing that turns up later as "the deploy worked but the machine is wrong", with no record anywhere of the warning that predicted it. Both handlers now go through reportToServer(), which takes the type as its first argument. The server (fogproject#1208) records either against the task in `taskLog` and in its own log, and fires HOST_IMAGE_FAIL only for an error -- announcing a failed deploy for a task that went on to succeed would be worse than saying nothing. The field carrying the message is `text` rather than `error` now that it is not always one. Nothing has shipped with the old name. The properties that mattered for handleError still hold, and now hold for handleWarning as well: time bounded, output discarded, exit status ignored, skipped entirely when there is no $web. tests/checks/error-report.sh grows three cases for the warning path and pins the type on both; all nine were mutation-verified against a build of the shipped library. Co-Authored-By: Claude <noreply@anthropic.com>
mastacontrola
changed the title
mastacontrola
added a commit
to FOGProject/fogproject
that referenced
this pull request
Aug 19, 2026
…hema 338) (#1208) #1207 gave FOS an endpoint to report an imaging failure to. It fired HOST_IMAGE_FAIL and called error_log(), and that was the whole of it -- so a report either reached a notification plugin or it reached the web server's error log, mixed in with everything else the web tier writes, correlated with nothing. Neither is a place an admin goes to ask "what did this task actually say before it stopped". A report now lands in three places: a `taskLog` row, typed and with the text in it. This is the one correlated with the task: it carries taskID and the state the task was in; /var/log/fog/fos/fosreports.log, listed by the Log Viewer like any other log because 'fos' is now in FOGLogPaths; HOST_IMAGE_FAIL, as before -- errors only, imaging tasks only. SCHEMA 338 adds `logType` (default 'state') and `logText` (NULL) to taskLog. Every row in that table so far is a state transition, which is exactly what the default backfills them as, so TaskingElement::taskLog() is untouched: a state row costs no extra column. logText is NULL rather than '' so "no body" and "an empty body" stay distinguishable. The step is a closure, like 336, because ADD COLUMN has no IF NOT EXISTS below MariaDB 10.0.2 and a re-run has to converge rather than error. TYPES. FOS now sends `type=error` or `type=warning`, because handleWarning() reports too (FOGProject/fos#152). A warning means the machine carried on, so it is recorded and it does not fire the failure event -- announcing a failed deploy for a task that went on to succeed would be worse than saying nothing. A report with no type at all is an error: an older FOS reporting a real failure must not be downgraded into silence. The row is written BEFORE the imaging gate, so a failed Memtest or inventory is recorded against its task even though no imaging event can fire for it. That gate is still there for the event itself -- firing HOST_IMAGE_FAIL for a non-imaging task is the defect #1202 removed, in the other direction. THE LOG FILE gets its own subdirectory, created by the installer as the web user with httpd_sys_rw_content_t. Both halves matter. It is not the top level because $servicelogs is root's and holds the eight daemons' logs, and rotation renames and unlinks -- the same split ADR 0010 made for the plugin runner. And it is not created by PHP, because /opt/fog inherits usr_t and httpd_t may READ usr_t but not write it (GH-964), so a directory made without the relabel looks right and silently swallows every write on an enforcing host. error_log() is kept as the fallback, not the destination: a server whose web tree is updated but which has not been re-installed yet has nowhere to write, and a report that reaches no log at all is the exact failure this path exists to end. Rotation keeps one old copy at SERVICE_LOG_SIZE -- the daemons' setting, so an admin who has already decided how big a FOG log may get does not decide again. One generation rather than five: this file gains a line per failed task, not a line per poll. VERIFIED against a throwaway copy of a real FOG database in a container, with the web tier served from a shadow tree, so nothing live was written: step 338 applied through its own closure, backfilled all 52 existing rows to 'state', and ran clean a second time; an error on a Deploy task wrote its row, its log line and fired HOST_IMAGE_FAIL to a registered listener; a warning on the same task wrote its row and its log line and fired nothing; an error on a Memtest task wrote its row and its log line and fired nothing; a report with no type was recorded as an error; an embedded newline arrived flattened to spaces, as #1207 intended; the file rotated to .1 once it passed SERVICE_LOG_SIZE. The `ip` column is empty in those rows only because filter_input(INPUT_SERVER) returns NULL under the cli-server SAPI the harness used; the same TaskLog constructor fills it under php-fpm, which the 52 pre-existing rows show. tests/task-error-report.test.php grows the type routing, the row's position relative to both gates, the model mapping, the FOGLogPaths entry and the installer's two lines. Every one of them was mutation-verified. Downstream: none. `tasklog` is already a Route::$validClasses entry, so OpenAPI picks the two columns up from the regenerated manifest. Co-authored-by: JJ Fullmer <7743340+darksidemilk@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
mastacontrola
pushed a commit
to FOGProject/fogproject
that referenced
this pull request
Aug 19, 2026
…ema 280) Ported from working-1.6 (#1207, #1208, #1209). This is a feature on a maintenance branch, and the reason it belongs here anyway is that FOS is not branched. FOGProject/fos#152 makes handleError() AND handleWarning() post a report to whatever server the machine booted from, and a 1.5 server is just as likely to be that server. Without this the POST reaches a 404 and 1.5 keeps the behaviour it has had since the beginning: a machine stops mid-image and says nothing to anyone. HOST_IMAGE_FAIL has two listeners in this tree (slack, pushbullet) and no core caller, so neither has ever fired on any 1.5 server. WHAT ARRIVES. service/taskerror.php takes mac, sysuuid, a type of error or warning, the text, and the script that raised it. A report lands in three places, none of which is the task's state: a `taskLog` row, typed, with the text in it -- the one correlated with the task, carrying taskID and the state the task was in; /var/log/fog/fos/fosreports.log, which the Log Viewer lists like any other; HOST_IMAGE_FAIL -- errors only, imaging tasks only. SCHEMA 280 adds `logType` (default 'state') and `logText` (NULL) to taskLog. Every row in that table so far is a state transition, which is what the default backfills them as, so TaskingElement::taskLog() is untouched. A closure, not a bare ALTER, because ADD COLUMN has no IF NOT EXISTS below MariaDB 10.0.2 and a re-run has to converge. A warning is recorded and fires nothing, because the machine carried on; announcing a failed deploy for a task that went on to succeed would be worse than silence. A report with NO type is an error -- on this branch that is the normal case rather than the exotic one, because a FOS newer than the server is what 1.5 will usually be talking to. THE INSTALLER PROBE HAD TO COME WITH IT, and this is the part that is not optional. installFOGDB() probes fogstorage's INSERT privilege with a throwaway row and reads any failure as "the grants need redoing", which is what makes it demand a database root password. That probe was positional: INSERT INTO taskLog VALUES ( 0, '999test', 3, '127.0.0.1', NOW(), 'fog'); Six values into what schema 280 makes an eight column table is error 1136, "Column count doesn't match value count" -- so this schema step alone would have made every 1.5 upgrade stop and ask for a root password nobody needs to type, on servers whose grants are perfectly correct. 1.6 shipped exactly that regression and had already been bitten once before by the same statement; see same commit as the cause rather than after it. THE LOG DIRECTORY is created by the installer as the web user with httpd_sys_rw_content_t, in its own subdirectory: $servicelogs is root's and holds the daemons' logs, and rotation renames and unlinks. error_log() stays as the fallback so a server updated but not yet re-installed still records something. 'fos' is added to all THREE lists 1.5 keeps -- StorageNode:: _getData(), status/getfiles.php and status/logtoview.php -- because they fail differently: miss the first two and the selector has no entry, miss the last and it answers "Invalid Folder". VERIFIED against a throwaway copy of a real 1.5 database (2079 hosts, schema 278) in a container, with the web tier served from a shadow tree, so the live 1.5 install was never written to: step 280 applied through its own closure, backfilled all 7 existing rows to 'state', and ran clean a second time; the OLD probe against the migrated table -> ERROR 1136, as predicted; the new one -> ROW_COUNT() = 1; error on a Deploy task -> row, log line, HOST_IMAGE_FAIL to a listener; warning on the same task -> row and log line, no event; no type at all -> recorded as an error, event fired; error on a non-imaging task -> row and log line, no event; an embedded newline arrived flattened to spaces; the file rotated to .1 once it passed SERVICE_LOG_SIZE. WHAT IS DELIBERATELY NOT PORTED. The slack and pushbullet listeners are left exactly as they are. 1.6's #1202 rewrote them to name the image and the reason; here they keep reading only $data['HostName'], and the extra payload keys are simply ignored. Making them fire at all is the change this branch needed; changing what they say is a separate one. Two tests, both mutation-verified: task-error-report.test.php (the sanitizer run for real against stubbed base classes, plus the type routing, the row's position relative to both gates, and all three log-path lists) and installer-db-probes-name-columns.test.php (any positional INSERT anywhere in the installer). Co-Authored-By: Claude <noreply@anthropic.com>
mastacontrola
added a commit
to FOGProject/fogproject
that referenced
this pull request
Aug 19, 2026
…ema 280) (#1210) Ported from working-1.6 (#1207, #1208, #1209). This is a feature on a maintenance branch, and the reason it belongs here anyway is that FOS is not branched. FOGProject/fos#152 makes handleError() AND handleWarning() post a report to whatever server the machine booted from, and a 1.5 server is just as likely to be that server. Without this the POST reaches a 404 and 1.5 keeps the behaviour it has had since the beginning: a machine stops mid-image and says nothing to anyone. HOST_IMAGE_FAIL has two listeners in this tree (slack, pushbullet) and no core caller, so neither has ever fired on any 1.5 server. WHAT ARRIVES. service/taskerror.php takes mac, sysuuid, a type of error or warning, the text, and the script that raised it. A report lands in three places, none of which is the task's state: a `taskLog` row, typed, with the text in it -- the one correlated with the task, carrying taskID and the state the task was in; /var/log/fog/fos/fosreports.log, which the Log Viewer lists like any other; HOST_IMAGE_FAIL -- errors only, imaging tasks only. SCHEMA 280 adds `logType` (default 'state') and `logText` (NULL) to taskLog. Every row in that table so far is a state transition, which is what the default backfills them as, so TaskingElement::taskLog() is untouched. A closure, not a bare ALTER, because ADD COLUMN has no IF NOT EXISTS below MariaDB 10.0.2 and a re-run has to converge. A warning is recorded and fires nothing, because the machine carried on; announcing a failed deploy for a task that went on to succeed would be worse than silence. A report with NO type is an error -- on this branch that is the normal case rather than the exotic one, because a FOS newer than the server is what 1.5 will usually be talking to. THE INSTALLER PROBE HAD TO COME WITH IT, and this is the part that is not optional. installFOGDB() probes fogstorage's INSERT privilege with a throwaway row and reads any failure as "the grants need redoing", which is what makes it demand a database root password. That probe was positional: INSERT INTO taskLog VALUES ( 0, '999test', 3, '127.0.0.1', NOW(), 'fog'); Six values into what schema 280 makes an eight column table is error 1136, "Column count doesn't match value count" -- so this schema step alone would have made every 1.5 upgrade stop and ask for a root password nobody needs to type, on servers whose grants are perfectly correct. 1.6 shipped exactly that regression and had already been bitten once before by the same statement; see same commit as the cause rather than after it. THE LOG DIRECTORY is created by the installer as the web user with httpd_sys_rw_content_t, in its own subdirectory: $servicelogs is root's and holds the daemons' logs, and rotation renames and unlinks. error_log() stays as the fallback so a server updated but not yet re-installed still records something. 'fos' is added to all THREE lists 1.5 keeps -- StorageNode:: _getData(), status/getfiles.php and status/logtoview.php -- because they fail differently: miss the first two and the selector has no entry, miss the last and it answers "Invalid Folder". VERIFIED against a throwaway copy of a real 1.5 database (2079 hosts, schema 278) in a container, with the web tier served from a shadow tree, so the live 1.5 install was never written to: step 280 applied through its own closure, backfilled all 7 existing rows to 'state', and ran clean a second time; the OLD probe against the migrated table -> ERROR 1136, as predicted; the new one -> ROW_COUNT() = 1; error on a Deploy task -> row, log line, HOST_IMAGE_FAIL to a listener; warning on the same task -> row and log line, no event; no type at all -> recorded as an error, event fired; error on a non-imaging task -> row and log line, no event; an embedded newline arrived flattened to spaces; the file rotated to .1 once it passed SERVICE_LOG_SIZE. WHAT IS DELIBERATELY NOT PORTED. The slack and pushbullet listeners are left exactly as they are. 1.6's #1202 rewrote them to name the image and the reason; here they keep reading only $data['HostName'], and the extra payload keys are simply ignored. Making them fire at all is the change this branch needed; changing what they say is a separate one. Two tests, both mutation-verified: task-error-report.test.php (the sanitizer run for real against stubbed base classes, plus the type routing, the row's position relative to both gates, and all three log-path lists) and installer-db-probes-name-columns.test.php (any positional INSERT anywhere in the installer). Co-authored-by: JJ Fullmer <7743340+darksidemilk@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
The FOS half of FOGProject/fogproject#1206. The server half is FOGProject/fogproject#1207 (merged), which adds the endpoint this posts to, and FOGProject/fogproject#1208, which gives a report somewhere to live and adds the
typethis now sends.Neither side needs the other to be safe: an older FOS sends no
typeand the server records it as an error; a newer FOS talking to an older server gets a 404 it already ignores.What was wrong
handleError()printed its banner and exited. It made no request to the server at all, so a real imaging failure — bad image, mount failure, partition error, any of thehandleErrorcall sites acrossfog.download,fog.upload,fog.mountand friends — was invisible to FOG.HOST_IMAGE_FAILcould not fire for it, and the notification plugins registered for that event (slack, ntfy, pushbullet) had never run on any server.The only failure FOG ever heard about was a storage node problem, via
fog.checkmount→blame.php, and that re-queues the task on another node rather than failing it.handleWarning()had the same hole and a worse one: it printed its banner, waited its minute and told nobody. A warning is the machine saying it hit something and carried on — a disk smaller than the image it is about to take, a partition table it had to work around — which is exactly what turns up later as "the deploy worked but the machine is wrong", with no record of the warning that predicted it.What this does
Both handlers go through
reportToServer(), which posts the message, the script that raised it, the identity FOS already sends everywhere else (mac,sysuuid), and which kind of report it is.The server records either against the task and in its own log, and fires
HOST_IMAGE_FAILonly for an error — announcing a failed deploy for a task that went on to succeed would be worse than saying nothing.The field carrying the message is
textrather thanerror, now that it is not always one. Nothing has shipped with the old name.Best effort, on purpose
This must not change anything the person standing in front of the machine sees or waits for:
--max-time 5, so an unreachable server cannot add to the wait before the reboot countdown;/dev/nulland|| :, so nothing prints and a non-zero exit cannot triperrexit;$webis unset (registration paths, debug shells);printf '%b'first, so the\nthe callers embed in their message becomes a real newline rather than the two literal characters. The server flattens control characters back to spaces at its end and bounds the length, so nothing here needs to.-Lksmatches every other call in this file — the FOG server's certificate is its own.Verification
The endpoint side is verified live in fogproject#1207, including the exact string shape
handleErrorproduces:arrives as one flattened line, with the reason preserved and the script name appended.
This side has
tests/checks/error-report.sh, nine cases following thetests/checks/sector-size.shpattern — sandbox copy of the library, PATH-shadowed stubs, and acurlstub that records its argv instead of making a request:Mutation-verified — hardcoding
type=error, droppinghandleWarning's call, renaming the field back toerror, dropping the$webguard, the output redirect,--max-time, theprintf %b, or the whole block each fails the harness.One caveat stated in the harness header rather than papered over: no FOS script sets
errexit, so the|| :is defensive and cannot be isolated by a case. Cases 5 and 9 pin the behaviour that matters instead — each handler still finishes when the report fails.Built and run in a real FOS image
./build.sh -nfa x64, exit 0. The built rootfs carries both call sites and today's stamp:The image was then extracted and run under podman (
--rootfs,--network host) against a FOG server carrying fogproject#1208 — so the shipped handlers ran with the shipped bash and the shipped curl, nothing from the host:What the server ended up with, from those two calls alone:
and exactly one
HOST_IMAGE_FAIL, for the error — the warning reached a registered listener not at all, which is the point.\nprintf %bhere, flattened to spaces by the server--max-time 5holds; without it this is where FOS would sit$webunsethandleWarningwith a reachable server added 0.13s to a function that already sleeps for sixty.The one thing not exercised is TLS: the shadow server was plain HTTP.
-Lkshere is identical to every other call in this file, andgetversion.phpandblame.phpalready reach a FOG server's self-signed certificate the same way, so this adds no new transport behaviour.Nothing was left behind — the build writes only to gitignored
dist/andfssource*/, the database was a throwaway container loaded from a dump (the live install was never written to), and the fixture host was removed.🤖 Generated with Claude Code
https://claude.ai/code/session_013mJVe4CpK3rRbi9H5GubXd