Skip to content

Say why imaging failed, not just that it did - #1226

Merged
mastacontrola merged 1 commit into
dev-branchfrom
imaging-failure-reason-dev
Aug 19, 2026
Merged

Say why imaging failed, not just that it did#1226
mastacontrola merged 1 commit into
dev-branchfrom
imaging-failure-reason-dev

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

TaskError has been sending ImageName and Reason on HOST_IMAGE_FAIL since the FOS reporting work landed — Reason being the flattened, MAX_REASON-bounded opening of whatever FOS actually reported. Both bundled listeners ignored all of it and pushed the fixed string This host has failed to image.

Found while doing the end-to-end live test of the FOS report path on 1.6. A report whose stored row reads

fog.download: failed to restore partition 2
  partclone.ntfs: /dev/sda2 is busy
  ERROR: win11-split part 2 checksum mismatch
  exit code 1 (fog.download)

produced a push saying, in its entirety, fos-deploy-test Failed / This host has failed to image. So the whole point of storing the trace — somebody seeing it — stopped at the server.

before after
slack Host: lab01 imaging failed. Host lab01 failed imaging Win11-Lab: fog.download: failed to restore partition 2 …
pushbullet lab01 Failed / This host has failed to image lab01 Imaging Failed / This host failed imaging Win11-Lab: fog.download: …

Defensive about every added key

These events fire only when something has already gone wrong, and a web tree can be older than whatever writes the payload. A bare $data['Reason'] would turn the notification into a PHP warning at the worst possible moment:

$reason = (string) ($data['Reason'] ?? '');
if ('' === $reason) {
    $reason = _('no reason was reported');
}

Both keys fall back to a translated placeholder rather than an empty slot, so an older server says failed imaging an unnamed image: no reason was reported instead of failed imaging : .

Translation

The substitution happens outside _(), with positional specifiers. A msgid built at runtime matches no catalog entry and never translates — silently and permanently — and %1$s lets a translator reorder the sentence.

Slack's old form had 'Host: %s ' outside the call and translated only the tail (_('imaging failed.')), which is not a sentence anyone could translate. That is now one whole msgid.

Relationship to 1.6

Ported from FOGProject/fog-plugins#21, which did the same for the 1.6 line, where these plugins now live as a pinned release asset. ntfy has no listener on this branch, so this is two files rather than three.

⚠️ The 1.6 side is merged but unreleasedfog-plugins main carries #21, the latest release v1.6.10 predates it, and FOG_PLUGINS_VERSION still pins v1.6.10. A v1.6.11 release and a pin bump are needed for 1.6 servers to get this; that is not part of this PR.

Verification

tests/imaging-failure-reason.test.php pins the reads, the defaults, the fallbacks and the gettext shape. Source-level, because these classes extend the plugin's Event base, which extends FOG's, so neither loads without a booted FOG, a session and a database.

Six mutations, all killed — and each confirmed to have actually applied to the file, rather than a no-op regex reading as a pass:

stop reading Reason              -> never reads Reason
read Reason with no default      -> read without a default
interpolate inside _()           -> msgid built at runtime
drop the missing-reason default  -> renders with an empty slot
bare %s instead of %1$s          -> translator cannot reorder
stop naming the image            -> never names the image

Full suite: 15 passed, 0 failed.

🤖 Generated with Claude Code

https://claude.ai/code/session_013mJVe4CpK3rRbi9H5GubXd

TaskError has been sending ImageName and Reason on HOST_IMAGE_FAIL since the
FOS reporting work landed -- Reason being the flattened, MAX_REASON-bounded
opening of whatever FOS actually reported. Both bundled listeners ignored all
of it and pushed the fixed string "This host has failed to image", which tells
an admin nothing the task list did not already show. Confirmed live on 1.6
before this: a report whose stored row read "fog.download: failed to restore
partition 2 / partclone.ntfs: /dev/sda2 is busy / ..." produced a push saying
only "Failed".

So the whole point of storing the trace -- somebody seeing it -- stopped at
the server.

    Host lab01 failed imaging Win11-Lab: fog.download: failed to restore
    partition 2 partclone.ntfs: /dev/sda2 is busy ...

Every added key is read defensively. These events fire only when something has
already gone wrong, and a web tree can be older than what writes the payload,
so a bare $data['Reason'] would turn the notification into a PHP warning at
the worst possible moment. Both keys fall back to a translated placeholder
rather than an empty slot, so an older server says "failed imaging an unnamed
image: no reason was reported" instead of "failed imaging : ".

The substitution happens OUTSIDE _(), with positional specifiers. A msgid
built at runtime matches no catalog entry and never translates, silently and
permanently; %1$s so a translator can reorder the sentence. Slack's old form
had 'Host: %s ' outside the call and translated only the tail, which is not a
sentence anyone could translate -- that is now one whole msgid.

Ported from FOGProject/fog-plugins#21, which did the same for the 1.6 line
where these plugins now live. ntfy has no listener on this branch, so this is
two files rather than three.

tests/imaging-failure-reason.test.php pins the reads, the defaults, the
fallbacks and the gettext shape -- source-level, because these classes extend
the plugin's Event base, which extends FOG's, so neither loads without a
booted FOG, a session and a database.

Six mutations, all killed, each confirmed to have actually applied:

    stop reading Reason              -> never reads Reason
    read Reason with no default      -> read without a default
    interpolate inside _()           -> msgid built at runtime
    drop the missing-reason default  -> renders with an empty slot
    bare %s instead of %1$s          -> translator cannot reorder
    stop naming the image            -> never names the image

Full suite: 15 passed, 0 failed.

Co-Authored-By: Claude <noreply@anthropic.com>
@mastacontrola
mastacontrola merged commit b7b748d into dev-branch Aug 19, 2026
3 checks passed
@mastacontrola
mastacontrola deleted the imaging-failure-reason-dev branch August 19, 2026 18:30
mastacontrola added a commit that referenced this pull request Aug 19, 2026
v1.6.11 carries FOGProject/fog-plugins#21, which was merged the morning after
v1.6.10 was cut and so has never reached a server.

That change matters more than its own PR made it sound, because the FOS
reporting work landed in between. #1206/#1211/#1217/#1223 give a failed task a
stored, multi-line report of what FOS actually said, and TaskError sends the
flattened opening of it as HOST_IMAGE_FAIL's Reason -- but every bundled
listener on v1.6.10 ignores that key and pushes the fixed string "This host
has failed to image".

Confirmed live before cutting the release. A report whose stored row read

    fog.download: failed to restore partition 2
      partclone.ntfs: /dev/sda2 is busy
      ERROR: win11-split part 2 checksum mismatch
      exit code 1 (fog.download)

pushed, in its entirety: "fos-deploy-test Failed" / "This host has failed to
image". So the whole point of storing the trace stopped at the server.

One line, because the sha256 is fetched from the release alongside the
tarball rather than pinned here; bin/fetch-plugins.sh reads this constant and
verifies what it downloads against it. Verified end to end on this pin:
"Fetching plugins v1.6.11" -> "Plugins at v1.6.11", and all three imagefail
listeners in the fetched tree read Reason.

The 1.5 line keeps its plugins in-tree and has no pin, so its equivalent is a
code change: #1226.

Co-authored-by: JJ Fullmer <7743340+darksidemilk@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants