fix: surface gh-pages publish errors via callback (#205) (#206)

fix: surface gh-pages publish errors via callback

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

Author: JohannesHoppe

src/engine/engine-filesystem.spec.ts

@@ -60,7 +60,12 @@ describe('engine - real filesystem tests', () => {
 
       const ghpages = require('gh-pages');
       jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
-      jest.spyOn(ghpages, 'publish').mockResolvedValue(undefined);
+      jest.spyOn(ghpages, 'publish').mockImplementation((_dir: unknown, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(null);
+        }
+        return Promise.resolve(undefined);
+      });
 
       const options = {
         notfound: true,
@@ -84,7 +89,12 @@ describe('engine - real filesystem tests', () => {
 
       const ghpages = require('gh-pages');
       jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
-      jest.spyOn(ghpages, 'publish').mockResolvedValue(undefined);
+      jest.spyOn(ghpages, 'publish').mockImplementation((_dir: unknown, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(null);
+        }
+        return Promise.resolve(undefined);
+      });
 
       const options = {
         notfound: false,
@@ -104,7 +114,12 @@ describe('engine - real filesystem tests', () => {
 
       const ghpages = require('gh-pages');
       jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
-      jest.spyOn(ghpages, 'publish').mockResolvedValue(undefined);
+      jest.spyOn(ghpages, 'publish').mockImplementation((_dir: unknown, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(null);
+        }
+        return Promise.resolve(undefined);
+      });
 
       const options = {
         notfound: true,
@@ -173,8 +188,11 @@ describe('engine - real filesystem tests', () => {
 
       let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
       const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
-        (dir: string, options: { cname?: string; nojekyll?: boolean }) => {
+        (_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
           capturedOptions = options;
+          if (callback) {
+            callback(null);
+          }
           return Promise.resolve();
         }
       );
@@ -202,8 +220,11 @@ describe('engine - real filesystem tests', () => {
 
       let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
       const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
-        (dir: string, options: { cname?: string; nojekyll?: boolean }) => {
+        (_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
           capturedOptions = options;
+          if (callback) {
+            callback(null);
+          }
           return Promise.resolve();
         }
       );
@@ -229,8 +250,11 @@ describe('engine - real filesystem tests', () => {
 
       let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
       const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
-        (dir: string, options: { cname?: string; nojekyll?: boolean }) => {
+        (_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
           capturedOptions = options;
+          if (callback) {
+            callback(null);
+          }
           return Promise.resolve();
         }
       );
@@ -263,8 +287,11 @@ describe('engine - real filesystem tests', () => {
 
       let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
       const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
-        (dir: string, options: { cname?: string; nojekyll?: boolean }) => {
+        (_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
           capturedOptions = options;
+          if (callback) {
+            callback(null);
+          }
           return Promise.resolve();
         }
       );
@@ -291,8 +318,11 @@ describe('engine - real filesystem tests', () => {
 
       let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
       const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
-        (dir: string, options: { cname?: string; nojekyll?: boolean }) => {
+        (_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
           capturedOptions = options;
+          if (callback) {
+            callback(null);
+          }
           return Promise.resolve();
         }
       );

src/engine/engine.gh-pages-behavior.spec.ts

@@ -1016,4 +1016,54 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
    * gh-pages.clean() behavior is tested in engine.gh-pages-clean.spec.ts
    * That test uses real filesystem operations without mocks.
    */
+
+  /**
+   * End-to-end canary for issue #205: drives the REAL gh-pages library
+   * through engine.run() with a mocked child_process.spawn that simulates
+   * an HTTPS auth failure during `git clone`. If upstream gh-pages ever
+   * fixes the `.then(_, onRejected)` absorption pattern, this test will
+   * still pass; if they change callback semantics, it will fail loudly.
+   */
+  describe('auth-failure silent-swallow regression (issue #205)', () => {
+    const engine = require('./engine');
+    const { cleanupMonkeypatch } = require('./engine.prepare-options-helpers');
+    const { logging } = require('@angular-devkit/core');
+
+    beforeEach(() => {
+      cleanupMonkeypatch();
+    });
+
+    it('engine.run() should reject when git clone fails with fatal: Authentication failed', async () => {
+      mockSpawn.mockImplementationOnce((cmd: string, args: string[] | undefined) => {
+        const capturedArgs = args || [];
+        spawnCalls.push({ cmd, args: capturedArgs, options: undefined });
+        const child = createMockChildProcess();
+        setImmediate(() => {
+          child.stderr!.emit(
+            'data',
+            Buffer.from(
+              "fatal: Authentication failed for 'https://github.com/owner/repo.git'"
+            )
+          );
+          child.emit!('close', 128);
+        });
+        return child;
+      });
+
+      const options = {
+        repo: 'https://x-access-token:bad-token@github.com/owner/repo.git',
+        branch: 'gh-pages',
+        dotfiles: true,
+        notfound: true,
+        nojekyll: true
+      };
+
+      await expect(
+        engine.run(basePath, options, new logging.NullLogger())
+      ).rejects.toThrow();
+
+      expect(spawnCalls[0]?.cmd).toBe('git');
+      expect(spawnCalls[0]?.args[0]).toBe('clone');
+    });
+  });
 });

src/engine/engine.gh-pages-integration.spec.ts

@@ -54,9 +54,14 @@ describe('engine - gh-pages integration', () => {
       ghpagesPublishSpy = jest.spyOn(ghpages, 'publish');
     }
 
-    // Set default mock implementations - gh-pages v5+ uses Promise-based API
+    // engine uses the callback form of gh-pages.publish() — see #205
     ghpagesCleanSpy.mockImplementation(() => {});
-    ghpagesPublishSpy.mockResolvedValue(undefined);
+    ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
+      if (callback) {
+        callback(null);
+      }
+      return Promise.resolve(undefined);
+    });
 
     // Create fresh copy of environment for each test
     // This preserves PATH, HOME, etc. needed by git
@@ -124,10 +129,11 @@ describe('engine - gh-pages integration', () => {
       await engine.run(testDir, options, logger);
 
       expect(ghpagesPublishSpy).toHaveBeenCalledTimes(1);
-      // gh-pages v5+ uses Promise-based API (no callback)
+      // engine uses the callback form of gh-pages.publish() — see #205
       expect(ghpagesPublishSpy).toHaveBeenCalledWith(
         testDir,
-        expect.any(Object)
+        expect.any(Object),
+        expect.any(Function)
       );
     });
 
@@ -515,23 +521,28 @@ describe('engine - gh-pages integration', () => {
   });
 
   describe('Promise handling integration', () => {
-    // gh-pages v5+ uses Promise-based API (we no longer use callback-based approach)
+    // engine uses the callback form of gh-pages.publish() — see #205
 
-    it('should invoke gh-pages.publish() without callback (Promise-based)', async () => {
+    it('should invoke gh-pages.publish() with a callback', async () => {
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };
 
       await engine.run(testDir, options, logger);
 
-      // gh-pages v5+ Promise API: publish(dir, options) - no callback
       expect(ghpagesPublishSpy).toHaveBeenCalledWith(
         expect.any(String),
-        expect.any(Object)
+        expect.any(Object),
+        expect.any(Function)
       );
     });
 
-    it('should resolve when gh-pages.publish() resolves', async () => {
-      ghpagesPublishSpy.mockResolvedValue(undefined);
+    it('should resolve when gh-pages.publish() invokes the callback with no error', async () => {
+      ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(null);
+        }
+        return Promise.resolve(undefined);
+      });
 
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };
@@ -541,9 +552,14 @@ describe('engine - gh-pages integration', () => {
       ).resolves.toBeUndefined();
     });
 
-    it('should reject when gh-pages.publish() rejects', async () => {
+    it('should reject when gh-pages.publish() invokes the callback with an error', async () => {
       const publishError = new Error('Git push failed');
-      ghpagesPublishSpy.mockRejectedValue(publishError);
+      ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(publishError);
+        }
+        return Promise.resolve(undefined);
+      });
 
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };
@@ -553,4 +569,57 @@ describe('engine - gh-pages integration', () => {
       ).rejects.toThrow('Git push failed');
     });
   });
+
+  describe('silent-swallow regression (issue #205)', () => {
+    // gh-pages@6 absorbs errors into its returned Promise via an internal
+    // .then(_, onRejected) handler that doesn't rethrow. The callback, however,
+    // still fires with the error. Engine must use the callback form so git
+    // failures (e.g. auth errors during clone) surface as rejections.
+
+    it('should reject when gh-pages resolves its promise but delivers an error via callback', async () => {
+      const authError = new Error(
+        "fatal: Authentication failed for 'https://github.com/owner/repo.git'"
+      );
+
+      ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(authError);
+        }
+        return Promise.resolve(undefined);
+      });
+
+      const testDir = '/test/dist';
+      const options = { dotfiles: true, notfound: true, nojekyll: true };
+
+      await expect(
+        engine.run(testDir, options, logger)
+      ).rejects.toThrow(/Authentication failed/);
+    });
+
+    it('should NOT log the success banner when publish fails via callback', async () => {
+      const authError = new Error('fatal: Authentication failed');
+
+      ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
+        if (callback) {
+          callback(authError);
+        }
+        return Promise.resolve(undefined);
+      });
+
+      const testLogger = new logging.Logger('test');
+      const infoSpy = jest.spyOn(testLogger, 'info');
+
+      const testDir = '/test/dist';
+      const options = { dotfiles: true, notfound: true, nojekyll: true };
+
+      await expect(
+        engine.run(testDir, options, testLogger)
+      ).rejects.toThrow();
+
+      const bannerCall = infoSpy.mock.calls.find(
+        ([msg]) => typeof msg === 'string' && msg.includes('Successfully published')
+      );
+      expect(bannerCall).toBeUndefined();
+    });
+  });
 });

src/engine/engine.spec.ts

@@ -279,11 +279,19 @@ describe('engine', () => {
       ghpagesPublishSpy.mockRestore();
     });
 
+    // engine uses the callback form of gh-pages.publish() — see #205
+    const mockPublishCallback = (error: Error | null) =>
+      (_dir: unknown, _opts: unknown, callback?: (err: Error | null) => void) => {
+        if (callback) {
+          callback(error);
+        }
+        return Promise.resolve(undefined);
+      };
+
     it('should reject when gh-pages.publish rejects with error', async () => {
       const publishError = new Error('Git push failed: permission denied');
 
-      // gh-pages v5+ properly rejects with errors
-      ghpagesPublishSpy.mockRejectedValue(publishError);
+      ghpagesPublishSpy.mockImplementation(mockPublishCallback(publishError));
 
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };
@@ -296,7 +304,7 @@ describe('engine', () => {
     it('should preserve error message through rejection', async () => {
       const detailedError = new Error('Remote url mismatch. Expected https://github.com/user/repo.git but got https://github.com/other/repo.git');
 
-      ghpagesPublishSpy.mockRejectedValue(detailedError);
+      ghpagesPublishSpy.mockImplementation(mockPublishCallback(detailedError));
 
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };
@@ -309,7 +317,7 @@ describe('engine', () => {
     it('should reject with authentication error from gh-pages', async () => {
       const authError = new Error('Authentication failed: Invalid credentials');
 
-      ghpagesPublishSpy.mockRejectedValue(authError);
+      ghpagesPublishSpy.mockImplementation(mockPublishCallback(authError));
 
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };
@@ -320,8 +328,7 @@ describe('engine', () => {
     });
 
     it('should resolve successfully when gh-pages.publish resolves', async () => {
-      // gh-pages v5+ resolves the Promise on success
-      ghpagesPublishSpy.mockResolvedValue(undefined);
+      ghpagesPublishSpy.mockImplementation(mockPublishCallback(null));
 
       const testDir = '/test/dist';
       const options = { dotfiles: true, notfound: true, nojekyll: true };

src/engine/engine.ts

@@ -197,7 +197,17 @@ async function publishViaGhPages(
     nojekyll: options.nojekyll
   };
 
-  // gh-pages v5+ fixed the Promise bug where errors didn't reject properly
-  // We can now safely await the promise directly
-  await ghPages.publish(dir, ghPagesOptions);
+  // gh-pages@6 silently absorbs errors via its internal .then(_, onRejected)
+  // handler (and returns undefined from some early-exit paths, upstream #465).
+  // The callback always fires with the error, so we bridge that to a rejection.
+  // See issue #205.
+  await new Promise<void>((resolve, reject) => {
+    ghPages.publish(dir, ghPagesOptions, (error: Error | null) => {
+      if (error) {
+        reject(error);
+        return;
+      }
+      resolve();
+    });
+  });
 }

src/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "angular-cli-ghpages",
-  "version": "3.0.2",
+  "version": "3.0.3",
   "description": "Deploy your Angular app to GitHub Pages or Cloudflare Pages directly from the Angular CLI (ng deploy)",
   "main": "index.js",
   "types": "index.d.ts",