fix: surface gh-pages publish errors via callback (#205)#206
Merged
JohannesHoppe merged 1 commit intomainfrom Apr 22, 2026
Merged
fix: surface gh-pages publish errors via callback (#205)#206JohannesHoppe merged 1 commit intomainfrom
JohannesHoppe merged 1 commit intomainfrom
Conversation
gh-pages@6 silently absorbs errors via its internal .then(_, onRejected)
handler that catches rejections and calls the user callback without
rethrowing — which converts the rejection into a fulfilment. The result:
git failures (notably HTTPS auth errors during clone) leave publish()'s
returned Promise in a resolved state, so angular-cli-ghpages printed its
"Successfully published" banner and exited 0 even when nothing was
pushed.
The v3.0.0 refactor removed a 2019 callback wrapper under the assumption
that upstream had fixed this. Upstream commit e1374b3 ("always return a
promise") only fixed two early-exit validation paths — not the main
chain's error absorption. Issues tschaub/gh-pages#465, #412 and #151
remain open.
Restore the callback-based wrapper: the callback always fires with the
error, so we bridge it to a rejection of our own Promise.
Regression tests:
- engine.gh-pages-integration.spec.ts: mock-contract test that encodes
the exact bug shape (callback fires with error, promise resolves),
plus a "no success banner on failure" assertion.
- engine.gh-pages-behavior.spec.ts: spawn-level canary that drives the
real gh-pages library through engine.run() with child_process.spawn
mocked to emit "fatal: Authentication failed" + exit 128 on clone.
Existing mocks across engine.spec.ts, engine-filesystem.spec.ts, and
parameter-tests/builder-integration.spec.ts updated to invoke the
callback (they previously relied on mockResolvedValue only and would
hang once engine started awaiting a callback).
Fixes #205
JohannesHoppe
added a commit
that referenced
this pull request
Apr 22, 2026
Prior tests covered the cleanup hook at the spawn-mock level only — they verified that the right git commands get issued, but not that the resulting gh-pages commit is actually clean. Add a real-filesystem, real-git test that: 1. Creates a local bare repo. 2. Seeds a gh-pages branch containing the exact conditions from #204: dotfiles (.gitignore, .gitmodules), nested dot-directory contents (.github/workflows/deploy.yml), a submodule gitlink, and a stale non-dot file. 3. Runs engine.run() (no mocks of child_process, fs, or gh-pages). 4. Inspects `git ls-tree -r gh-pages` on the bare repo and asserts the final tree contains ONLY index.html. Also adds a baseline test that calls gh-pages.publish() directly (bypassing our hook) and asserts the dotfiles + submodule DO leak — i.e. the upstream bug is real on the installed gh-pages version. If a future gh-pages release fixes the bug, that baseline test will fail as a clear signal that our workaround can be removed. No production changes; the hook implementation and its PR #206-era callback wrapper are exercised end-to-end by the new tests.
JohannesHoppe
added a commit
that referenced
this pull request
Apr 22, 2026
* fix: clean up leftover gh-pages branch files (dotfiles, submodules) gh-pages@6.3.0's "Removing files" step calls globby without `dot: true`, so dotfiles (.gitignore, .gitmodules, .github/) and submodule gitlinks from the gh-pages branch are not removed before our dist is copied on top. They then get re-committed and leak into the deploy. Upstream fix tschaub/gh-pages#612 (merged 2025-08-09) adds both `remove: '**/*'` and `dot: true`, but is unreleased as of v6.3.0. We can't reach the globby call from options alone. Workaround: register a `beforeAdd` hook that runs after gh-pages' broken remove + our file copy. The hook asks git what it still has indexed (`git ls-files -z`), diffs against the set of files in our dist, and `git rm`s the leftovers. `git rm` handles submodule gitlinks correctly, so the `build` gitlink from the reporter's scenario is also cleaned up. Skipped when the user opts into `add: true` (additive mode explicitly preserves existing files). Adds a spawn-level regression test in engine.gh-pages-behavior.spec.ts that mocks `git ls-files` to return leftover dotfiles + a submodule gitlink, runs engine.run() end-to-end through real gh-pages, and asserts `git rm` targets the leftovers while leaving dist files alone. Fixes #204 * test: add end-to-end real-git integration test for #204 cleanup Prior tests covered the cleanup hook at the spawn-mock level only — they verified that the right git commands get issued, but not that the resulting gh-pages commit is actually clean. Add a real-filesystem, real-git test that: 1. Creates a local bare repo. 2. Seeds a gh-pages branch containing the exact conditions from #204: dotfiles (.gitignore, .gitmodules), nested dot-directory contents (.github/workflows/deploy.yml), a submodule gitlink, and a stale non-dot file. 3. Runs engine.run() (no mocks of child_process, fs, or gh-pages). 4. Inspects `git ls-tree -r gh-pages` on the bare repo and asserts the final tree contains ONLY index.html. Also adds a baseline test that calls gh-pages.publish() directly (bypassing our hook) and asserts the dotfiles + submodule DO leak — i.e. the upstream bug is real on the installed gh-pages version. If a future gh-pages release fixes the bug, that baseline test will fail as a clear signal that our workaround can be removed. No production changes; the hook implementation and its PR #206-era callback wrapper are exercised end-to-end by the new tests.
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
1 participant
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.
Summary
Fixes #205 — silent-swallow of
gh-pagespublish errors. CI builds were printing a green "Successfully published" banner and exiting 0 even whengit clonefailed withfatal: Authentication failed(e.g., expiredGH_TOKEN), causing a full month of stale deploys in at least one reported setup.ghPages.publish()insrc/engine/engine.ts. The.then(_, onRejected)handler inside gh-pages absorbs rejections (it calls the user callback without rethrowing), so the returned Promise resolves even on failure. The callback, however, still fires reliably with the error — bridging it to a Promise rejection is the fix.child_process.spawn).3.0.2 → 3.0.3.Root cause
In v3.0.0 (#200) the historical callback wrapper was removed with the comment "gh-pages v5+ fixed the Promise bug". That referred to upstream commit
e1374b3("always return a promise") — but that commit only fixed two early-exit validation paths (basePathnot a directory /statSyncthrows). The main chain atgh-pages/lib/index.js:254-264still absorbs rejections:Upstream issues tschaub/gh-pages#465, tschaub/gh-pages#412, tschaub/gh-pages#151 are all still open tracking variants of this.
--> Reverting to the previous workaround from the v2 branch.
Note on tests
The spawn-level canary in
engine.gh-pages-behavior.spec.tsexercises the real gh-pages library. If a future gh-pages release ever fixes the.then(_, onRejected)absorption, this test will still pass — giving a safer signal for later removing the wrapper than relying on the commit-message claim alone.