SP-2668: Allow profile recovery when the default profile cannot refresh

src/commands/git-profile/module.ts

@@ -15,7 +15,8 @@ class Module extends IModule {
                 .description("Commands related to Git settings");
 
         const gitProfileCommand = gitCommand.command("profile")
-            .description("Manage Git profiles required to use git-related operations.");
+            .description("Manage Git profiles required to use git-related operations.")
+            .skipProfileLoading();
 
         gitProfileCommand.command("list")
                 .description("Command to list all stored Git profiles")

src/commands/profile/module.ts

@@ -12,7 +12,8 @@ class Module extends IModule {
 
     public register(context: Context, configurator: Configurator): void {
         const command = configurator.command("profile")
-                .description("Manage profiles required to access a system.");
+                .description("Manage profiles required to access a system.")
+                .skipProfileLoading();
 
         command.command("list")
                 .description("Command to list all stored profiles")

src/content-cli.ts

@@ -2,7 +2,7 @@
 
 import semverSatisfies = require("semver/functions/satisfies");
 import { Command } from "commander";
-import { Configurator, IModuleConstructor, ModuleHandler } from "./core/command/module-handler";
+import { Configurator, IModuleConstructor, ModuleHandler, shouldLoadProfile } from "./core/command/module-handler";
 import { Context } from "./core/command/cli-context";
 import { VersionUtils } from "./core/utils/version";
 import { logger } from "./core/utils/logger";
@@ -63,6 +63,14 @@ export function createProgram(context: Context, opts: CreateProgramOptions = {})
     return program;
 }
 
+export function loadProfileOnDemand(program: Command, context: Context): void {
+    program.hook("preAction", async (_program, actionCommand) => {
+        if (shouldLoadProfile(actionCommand)) {
+            await context.init();
+        }
+    });
+}
+
 /**
  * To support the legacy command structure, we have to configure some root commands
  * that the individual modules will extend.
@@ -103,9 +111,8 @@ async function run(): Promise<void> {
     }
 
     const context = new Context(globalOpts);
-    await context.init();
-
     const program = createProgram(context, { devMode: !!globalOpts.dev });
+    loadProfileOnDemand(program, context);
 
     try {
         await program.parseAsync(process.argv);

src/core/command/module-handler.ts

@@ -123,6 +123,15 @@ export class ModuleHandler {
 
 type CommandHandler = (context: Context, command: Command, options: OptionValues) => Promise<void>;
 
+export function shouldLoadProfile(command: Command): boolean {
+    for (let cmd = command; cmd; cmd = cmd.parent) {
+        if ((cmd as any).skipsProfileLoading) {
+            return false;
+        }
+    }
+    return true;
+}
+
 /**
  * Allows the creation of root level commands.
  */
@@ -209,6 +218,11 @@ export class CommandConfig {
         return this;
     }
 
+    public skipProfileLoading(): this {
+        (this.cmd as any).skipsProfileLoading = true;
+        return this;
+    }
+
     public action(handler: CommandHandler): void {
         this.cmd.action(async (): Promise<void> => {
             try {

src/core/profile/profile.service.ts

@@ -19,7 +19,6 @@ const expiryBuffer = 5000;
 const OAUTH_SCOPES = ["studio", "package-manager", "integration.data-pools", "action-engine.projects"];
 /** Device code fallback: try without action-engine.projects if all 4 scopes fail. */
 const DEVICE_CODE_SCOPES_WITHOUT_ACTION_ENGINE = ["studio", "package-manager", "integration.data-pools"];
-
 export interface Config {
     defaultProfile: string;
 }
@@ -263,9 +262,9 @@ export class ProfileService {
         if (!this.isProfileExpired(profile, expiryBuffer)) {
             return;
         }
-        const issuer = await Issuer.discover(profile.team);
-        if (profile.type === ProfileType.DEVICE_CODE) {
-            try {
+        try {
+            const issuer = await Issuer.discover(profile.team);
+            if (profile.type === ProfileType.DEVICE_CODE) {
                 const oauthClient = new issuer.Client({
                     client_id: "content-cli",
                     token_endpoint_auth_method: "none",
@@ -274,12 +273,7 @@ export class ProfileService {
                 profile.apiToken = tokenSet.access_token;
                 profile.expiresAt = tokenSet.expires_at;
                 profile.refreshToken = tokenSet.refresh_token;
-            } catch (err) {
-                logger.error(new FatalError("The profile cannot be refreshed. Please retry or recreate profile."));
-            }
-        }
-        else {
-            try {
+            } else {
                 const oauthClient = new issuer.Client({
                     client_id: profile.clientId,
                     client_secret: profile.clientSecret,
@@ -291,14 +285,21 @@ export class ProfileService {
                 });
                 profile.apiToken = tokenSet.access_token;
                 profile.expiresAt = tokenSet.expires_at;
-            } catch (err) {
-                logger.error(new FatalError("The profile cannot be refreshed. Please retry or recreate profile."));
             }
+        } catch (err) {
+            throw this.unrefreshableProfile(profile.name, err);
         }
 
         await this.storeProfile(profile);
     }
 
+    private unrefreshableProfile(profileName: string, err: unknown): Error {
+        const cause = err instanceof Error ? err.message : "an unrecognised error";
+        logger.debug(`Refreshing the profile ${profileName} failed: ${cause}`);
+        logger.warn(`The profile ${profileName} cannot be refreshed. Retry, or recreate it.`);
+        return new Error(`The profile ${profileName} cannot be refreshed.`);
+    }
+
     private getProfileEnvVariables(): any {
         return {
             teamUrl: this.getBaseTeamUrl(process.env.TEAM_URL),

tests/commands/profile/profile-recovery.spec.ts

@@ -0,0 +1,93 @@
+import Module = require("../../../src/commands/profile/module");
+import { createProgram, loadProfileOnDemand } from "../../../src/content-cli";
+import { ProfileCommandService } from "../../../src/commands/profile/profile-command.service";
+import { Context } from "../../../src/core/command/cli-context";
+import { GitProfileService } from "../../../src/core/git-profile/git-profile.service";
+import { ProfileService } from "../../../src/core/profile/profile.service";
+
+jest.mock("../../../src/commands/profile/profile-command.service");
+
+describe("Profile commands when the default profile cannot be refreshed", () => {
+
+    let exitSpy: jest.SpyInstance;
+    let findProfile: jest.SpyInstance;
+    let listProfiles: jest.Mock;
+    let createProfile: jest.Mock;
+    let defaultProfile: jest.Mock;
+    let secureProfile: jest.Mock;
+
+    beforeEach(() => {
+        exitSpy = jest.spyOn(process, "exit").mockImplementation((() => undefined) as never);
+
+        jest.spyOn(GitProfileService.prototype, "getDefaultProfile").mockReturnValue(null);
+        jest.spyOn(GitProfileService.prototype, "findProfile").mockRejectedValue(new Error("No profile was found"));
+        jest.spyOn(ProfileService.prototype, "getDefaultProfile").mockReturnValue("expired-profile");
+        findProfile = jest.spyOn(ProfileService.prototype, "findProfile")
+            .mockRejectedValue("The profile expired-profile couldn't be resolved.");
+
+        listProfiles = jest.fn().mockResolvedValue(undefined);
+        createProfile = jest.fn().mockResolvedValue(undefined);
+        defaultProfile = jest.fn().mockResolvedValue(undefined);
+        secureProfile = jest.fn().mockResolvedValue(undefined);
+        (ProfileCommandService as jest.MockedClass<typeof ProfileCommandService>).mockImplementation(() => ({
+            listProfiles,
+            createProfile,
+            makeDefaultProfile: defaultProfile,
+            secureProfile,
+        }) as unknown as ProfileCommandService);
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
+    async function runProfileCommand(args: string[]): Promise<Context> {
+        const context = new Context({});
+
+        const program = createProgram(context, { modules: [Module] });
+        loadProfileOnDemand(program, context);
+        program.exitOverride();
+        await program.parseAsync(["node", "content-cli", ...args]);
+
+        return context;
+    }
+
+    it("should run profile list without loading a profile", async () => {
+        await runProfileCommand(["profile", "list"]);
+
+        expect(listProfiles).toHaveBeenCalled();
+        expect(findProfile).not.toHaveBeenCalled();
+        expect(exitSpy).not.toHaveBeenCalled();
+    });
+
+    it("should run profile create without loading a profile", async () => {
+        await runProfileCommand(["profile", "create", "--setAsDefault"]);
+
+        expect(createProfile).toHaveBeenCalledWith(true);
+        expect(findProfile).not.toHaveBeenCalled();
+        expect(exitSpy).not.toHaveBeenCalled();
+    });
+
+    it("should run profile default without loading a profile", async () => {
+        await runProfileCommand(["profile", "default", "replacement"]);
+
+        expect(defaultProfile).toHaveBeenCalledWith("replacement");
+        expect(findProfile).not.toHaveBeenCalled();
+        expect(exitSpy).not.toHaveBeenCalled();
+    });
+
+    it("should run profile secure without loading a profile", async () => {
+        await runProfileCommand(["profile", "secure", "broken"]);
+
+        expect(secureProfile).toHaveBeenCalledWith("broken");
+        expect(findProfile).not.toHaveBeenCalled();
+        expect(exitSpy).not.toHaveBeenCalled();
+    });
+
+    it("should leave the context without a profile so content commands still fail", async () => {
+        const context = await runProfileCommand(["profile", "list"]);
+
+        expect(context.profile).toBeUndefined();
+        expect(() => context.httpClient).toThrow("No profile provided");
+    });
+});

tests/core/command/profile-on-demand.spec.ts

@@ -0,0 +1,79 @@
+import { Command } from "commander";
+import { createProgram, loadProfileOnDemand } from "../../../src/content-cli";
+import { Context } from "../../../src/core/command/cli-context";
+import { Configurator, IModule, shouldLoadProfile } from "../../../src/core/command/module-handler";
+
+class ProfiledModule extends IModule {
+    public register(context: Context, configurator: Configurator): void {
+        configurator.command("content")
+            .command("show")
+            .action(async () => undefined);
+    }
+}
+
+class ProfileFreeModule extends IModule {
+    public register(context: Context, configurator: Configurator): void {
+        configurator.command("settings")
+            .skipProfileLoading()
+            .command("show")
+            .action(async () => undefined);
+    }
+}
+
+describe("shouldLoadProfile", () => {
+
+    it("should load a profile for an unmarked command", () => {
+        const command = new Command("export");
+
+        expect(shouldLoadProfile(command)).toBe(true);
+    });
+
+    it("should not load a profile for a marked command", () => {
+        const command = new Command("profile");
+        (command as any).skipsProfileLoading = true;
+
+        expect(shouldLoadProfile(command)).toBe(false);
+    });
+
+    it("should inherit the opt-out from an ancestor", () => {
+        const parent = new Command("profile");
+        (parent as any).skipsProfileLoading = true;
+        const child = parent.command("list");
+
+        expect(shouldLoadProfile(child)).toBe(false);
+    });
+});
+
+describe("loadProfileOnDemand", () => {
+
+    let context: Context;
+    let init: jest.SpyInstance;
+
+    beforeEach(() => {
+        context = new Context({});
+        init = jest.spyOn(context, "init").mockResolvedValue(undefined);
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
+    async function run(modules: any[], args: string[]): Promise<void> {
+        const program = createProgram(context, { modules });
+        loadProfileOnDemand(program, context);
+        program.exitOverride();
+        await program.parseAsync(["node", "content-cli", ...args]);
+    }
+
+    it("should load the profile for a command that needs one", async () => {
+        await run([ProfiledModule], ["content", "show"]);
+
+        expect(init).toHaveBeenCalledTimes(1);
+    });
+
+    it("should not load the profile for a command that opted out", async () => {
+        await run([ProfileFreeModule], ["settings", "show"]);
+
+        expect(init).not.toHaveBeenCalled();
+    });
+});

tests/core/profile/profile-refresh-exit.spec.ts

@@ -0,0 +1,65 @@
+const mockIssuerDiscover = jest.fn();
+
+jest.mock("openid-client", () => ({
+    Issuer: { discover: mockIssuerDiscover },
+}));
+
+import { AuthenticationType, Profile, ProfileType } from "../../../src/core/profile/profile.interface";
+import { ProfileService } from "../../../src/core/profile/profile.service";
+
+describe("ProfileService - refreshProfile keeps the process alive", () => {
+
+    let profileService: ProfileService;
+    let exitSpy: jest.SpyInstance;
+    let storeSpy: jest.SpyInstance;
+
+    const expiredProfile = (): Profile => ({
+        name: "broken",
+        team: "https://example.invalid",
+        apiToken: "stale-token",
+        refreshToken: "revoked-refresh-token",
+        authenticationType: AuthenticationType.BEARER,
+        type: ProfileType.DEVICE_CODE,
+        expiresAt: Math.floor(Date.now() / 1000) - 10,
+    });
+
+    beforeEach(() => {
+        profileService = new ProfileService();
+        exitSpy = jest.spyOn(process, "exit").mockImplementation((() => undefined) as never);
+        storeSpy = jest.spyOn(profileService, "storeProfile").mockImplementation(async () => {});
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+        mockIssuerDiscover.mockReset();
+    });
+
+    it("should reject without exiting when the token endpoint rejects the refresh token", async () => {
+        mockIssuerDiscover.mockResolvedValue({
+            Client: jest.fn().mockImplementation(() => ({
+                refresh: jest.fn().mockRejectedValue(new Error("invalid_grant")),
+            })),
+        });
+
+        await expect(profileService.refreshProfile(expiredProfile()))
+            .rejects.toThrow("The profile broken cannot be refreshed.");
+
+        expect(exitSpy).not.toHaveBeenCalled();
+        expect(storeSpy).not.toHaveBeenCalled();
+    });
+
+    it("should reject without exiting when the team is unreachable", async () => {
+        const dnsFailure = Object.assign(new Error("getaddrinfo ENOTFOUND example.invalid"), {
+            errno: -3008,
+            code: "ENOTFOUND",
+            syscall: "getaddrinfo",
+        });
+        mockIssuerDiscover.mockRejectedValue(dnsFailure);
+
+        await expect(profileService.refreshProfile(expiredProfile()))
+            .rejects.toThrow("The profile broken cannot be refreshed.");
+
+        expect(exitSpy).not.toHaveBeenCalled();
+        expect(storeSpy).not.toHaveBeenCalled();
+    });
+});