Skip to content

Commit 4331cf4

Browse files
jace-roelltraeok
andauthored
Extender retry (#3830)
* init Signed-off-by: jace-roell <[email protected]> * setting configuration Signed-off-by: jace-roell <[email protected]> * refactor retry Signed-off-by: jace-roell <[email protected]> * test updates Signed-off-by: jace-roell <[email protected]> * fix dataset provider function buff builder location Signed-off-by: jace-roell <[email protected]> * ds text fix Signed-off-by: jace-roell <[email protected]> * remove ds handleAuthCall Signed-off-by: jace-roell <[email protected]> * fix fs tests Signed-off-by: jace-roell <[email protected]> * usstree fix Signed-off-by: jace-roell <[email protected]> * fix zoom test Signed-off-by: jace-roell <[email protected]> * auth handler init test Signed-off-by: jace-roell <[email protected]> * test Signed-off-by: jace-roell <[email protected]> * update main Signed-off-by: jace-roell <[email protected]> * patch coverage Signed-off-by: jace-roell <[email protected]> * changelog Signed-off-by: jace-roell <[email protected]> * test fix and unused map Signed-off-by: jace-roell <[email protected]> * fix broken mock Signed-off-by: jace-roell <[email protected]> * fix Signed-off-by: jace-roell <[email protected]> * remove only Signed-off-by: jace-roell <[email protected]> * unused import Signed-off-by: jace-roell <[email protected]> * move maxExtenderRetry to a setting instead of under table Signed-off-by: jace-roell <[email protected]> * track prompts by profile Signed-off-by: jace-roell <[email protected]> * rework to handle no profile object on profile uri Signed-off-by: jace-roell <[email protected]> * remove log statement Signed-off-by: jace-roell <[email protected]> * fix token check Signed-off-by: jace-roell <[email protected]> * init Signed-off-by: jace-roell <[email protected]> * fix inital auth prompt Signed-off-by: jace-roell <[email protected]> * test Signed-off-by: jace-roell <[email protected]> * fix void for init workspace Signed-off-by: jace-roell <[email protected]> * implement comment fixes, search after sso login Signed-off-by: jace-roell <[email protected]> * sso login after search dataset filter Signed-off-by: jace-roell <[email protected]> * shared init test fix Signed-off-by: jace-roell <[email protected]> * check before changing node collapisbleState Signed-off-by: jace-roell <[email protected]> * existing test fixes Signed-off-by: jace-roell <[email protected]> * patch cov Signed-off-by: jace-roell <[email protected]> * log statement Signed-off-by: jace-roell <[email protected]> * remove auth type none check Signed-off-by: jace-roell <[email protected]> * unused import Signed-off-by: jace-roell <[email protected]> * duplicate prompt Signed-off-by: jace-roell <[email protected]> * changelog Signed-off-by: jace-roell <[email protected]> * direct FS calls check profile type for auth prompt Signed-off-by: jace-roell <[email protected]> * remove timeout by default Signed-off-by: jace-roell <[email protected]> * address feedback Signed-off-by: jace-roell <[email protected]> * update setting name to Max Request Retry Signed-off-by: jace-roell <[email protected]> * unused import Signed-off-by: jace-roell <[email protected]> * awaitTimeout removal for some UssFs calls, retry wait, handleAuthError dont wait Signed-off-by: jace-roell <[email protected]> * test fixes Signed-off-by: jace-roell <[email protected]> * test commit Signed-off-by: jace-roell <[email protected]> * refactor: Wait for active request to resolve auth prompt Signed-off-by: Trae Yelovich <[email protected]> * update prompting behavior in FS to adhere to prompt description Signed-off-by: Trae Yelovich <[email protected]> * fix: remove extra lockProfile call in retry logic Signed-off-by: Trae Yelovich <[email protected]> * respect retry count Signed-off-by: jace-roell <[email protected]> * change timeout to 30s Signed-off-by: jace-roell <[email protected]> * wip(refactor): Queue FS requests until one succeeds for a profile Signed-off-by: Trae Yelovich <[email protected]> * fix: finish activating before fetching workspaces Signed-off-by: Trae Yelovich <[email protected]> * refactor: skip .vscode calls in data set FS Signed-off-by: Trae Yelovich <[email protected]> * fix failing tests Signed-off-by: jace-roell <[email protected]> * add tests & typedoc for sequential/parallel handling Signed-off-by: Trae Yelovich <[email protected]> * fix: add preceding slash to .vscode check in DS FS Signed-off-by: Trae Yelovich <[email protected]> * refactor: release queued reqs after parallel mode re-enabled Signed-off-by: Trae Yelovich <[email protected]> * address lint errors Signed-off-by: Trae Yelovich <[email protected]> * refactor: waitForUnlock no longer times out >30s Signed-off-by: Trae Yelovich <[email protected]> Co-authored-by: Jace Roell <[email protected]> * wip: remove request retry setting Signed-off-by: jace-roell <[email protected]> * remove try catch on canceled prompt Signed-off-by: jace-roell <[email protected]> * remove check for state of authentication - only consider cancellation Signed-off-by: jace-roell <[email protected]> * fix merge conflict and unused import Signed-off-by: jace-roell <[email protected]> * remove duplication in runSequentialIfEnabled catch block Signed-off-by: Trae Yelovich <[email protected]> * fix workspace extra prmopts Signed-off-by: jace-roell <[email protected]> * changelog update Signed-off-by: jace-roell <[email protected]> * fix changelog Signed-off-by: jace-roell <[email protected]> --------- Signed-off-by: jace-roell <[email protected]> Signed-off-by: Jace Roell <[email protected]> Signed-off-by: Trae Yelovich <[email protected]> Co-authored-by: Trae Yelovich <[email protected]>
1 parent 4b6f192 commit 4331cf4

File tree

23 files changed

+1304
-618
lines changed

23 files changed

+1304
-618
lines changed

packages/zowe-explorer-api/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t
1111

1212
### Bug fixes
1313

14+
- Fixed an issue where the `promptForAuthentication` function would throw an `AuthCanceledError` on invalid authentication for SSO login. [#3830](https://github.com/zowe/zowe-explorer-vscode/pull/3830)
1415
- Fixed an issue where secure credentials and headers were being logged to the Zowe logger and VSCode output channel. [#3848](https://github.com/zowe/zowe-explorer-vscode/pull/3848)
1516
- Added support to delete VSAM data sets for z/OSMF type profiles. [#3824](https://github.com/zowe/zowe-explorer-vscode/issues/3824)
1617
- Fixed an issue where the `IMvs.putContents` function did not properly strip carriage returns from chunks during upload operations on Windows. Now, the function correctly converts CRLF sequences to LF, even if the sequence lands on a chunk boundary. [#3853](https://github.com/zowe/zowe-explorer-vscode/issues/3853)

packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts

Lines changed: 168 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import { Mutex } from "async-mutex";
1313
import { AuthHandler, AuthCancelledError, Gui, ZoweVsCodeExtension } from "../../../src";
1414
import { FileManagement } from "../../../src/utils/FileManagement";
15-
import { ImperativeError, IProfileLoaded, Session, SessConstants } from "@zowe/imperative";
15+
import { ImperativeError, IProfileLoaded, Session, SessConstants, RestConstants } from "@zowe/imperative";
1616
import { AuthPromptParams } from "../../../src/profiles/AuthHandler";
1717
import * as vscode from "vscode";
1818

@@ -40,6 +40,7 @@ describe("AuthHandler", () => {
4040
beforeEach(() => {
4141
// Since wasAuthCancelled relies on internal state, we clear it.
4242
(AuthHandler as any).authCancelledProfiles.clear();
43+
(AuthHandler as any).authFlows.clear();
4344
});
4445

4546
it("should return true when auth has been cancelled for a profile name", () => {
@@ -146,12 +147,10 @@ describe("AuthHandler", () => {
146147
},
147148
imperativeError,
148149
};
149-
const releaseSpy = jest.spyOn(Mutex.prototype, "release");
150150
const result = await AuthHandler.lockProfile(TEST_PROFILE_NAME, authOpts);
151-
expect(result).toBe(true);
151+
expect(result).toBe(false);
152152
expect(promptForAuthenticationMock).toHaveBeenCalledTimes(1);
153153
expect(promptForAuthenticationMock).toHaveBeenCalledWith(TEST_PROFILE_NAME, authOpts);
154-
expect(releaseSpy).toHaveBeenCalledTimes(1);
155154
AuthHandler.unlockProfile(TEST_PROFILE_NAME);
156155
});
157156

@@ -362,13 +361,16 @@ describe("AuthHandler", () => {
362361
expect(releaseSpy).not.toHaveBeenCalled();
363362
});
364363

365-
it("does nothing if the mutex in the map is not locked", async () => {
364+
it("deletes auth flow and releases auth prompt lock when unlocking", async () => {
365+
// Setup mutexes for the profile
366366
await AuthHandler.lockProfile(TEST_PROFILE_NAME);
367367
AuthHandler.unlockProfile(TEST_PROFILE_NAME);
368368

369-
const releaseSpy = jest.spyOn(Mutex.prototype, "release").mockClear();
370369
AuthHandler.unlockProfile(TEST_PROFILE_NAME);
371-
expect(releaseSpy).not.toHaveBeenCalled();
370+
const authPromptLock = AuthHandler.authPromptLocks.get(TEST_PROFILE_NAME);
371+
expect(authPromptLock).not.toBeUndefined();
372+
expect(authPromptLock.isLocked()).toBe(false);
373+
expect(AuthHandler.getActiveAuthFlow(TEST_PROFILE_NAME)).toBe(undefined);
372374
});
373375

374376
it("reuses the same Mutex for the profile if it already exists", async () => {
@@ -394,12 +396,166 @@ describe("AuthHandler", () => {
394396
});
395397
});
396398

397-
describe("shouldHandleAuthError", () => {
398-
it("returns true if a credential prompt was not yet shown to the user", async () => {
399-
await expect(AuthHandler.shouldHandleAuthError(TEST_PROFILE_NAME)).resolves.toBe(true);
399+
describe("getOrCreateAuthFlow", () => {
400+
const authOpts: AuthPromptParams = {
401+
authMethods: {
402+
promptCredentials: jest.fn(),
403+
ssoLogin: jest.fn(),
404+
},
405+
imperativeError: new ImperativeError({
406+
msg: "Username or password is invalid or expired",
407+
errorCode: RestConstants.HTTP_STATUS_401.toString(),
408+
}),
409+
};
410+
411+
beforeEach(() => {
412+
(AuthHandler as any).authFlows.clear();
413+
(AuthHandler as any).authPromptLocks.clear();
414+
(AuthHandler as any).profileLocks.clear();
415+
});
416+
417+
it("reuses the same promise for concurrent requests", async () => {
418+
let resolveFlow: ((value: boolean | PromiseLike<boolean>) => void) | undefined;
419+
const lockProfileMock = jest.spyOn(AuthHandler, "lockProfile").mockImplementation(
420+
() =>
421+
new Promise<boolean>((resolve) => {
422+
resolveFlow = resolve;
423+
})
424+
);
425+
426+
const flowOne = AuthHandler.getOrCreateAuthFlow(TEST_PROFILE_NAME, authOpts);
427+
const flowTwo = AuthHandler.getOrCreateAuthFlow(TEST_PROFILE_NAME, authOpts);
428+
429+
expect(flowOne).toStrictEqual(flowTwo);
430+
expect(lockProfileMock).toHaveBeenCalledTimes(1);
431+
432+
resolveFlow?.(true);
433+
await expect(flowOne).resolves.toBeUndefined();
434+
expect(AuthHandler.getActiveAuthFlow(TEST_PROFILE_NAME)).toBeUndefined();
435+
lockProfileMock.mockRestore();
436+
});
437+
438+
it("clears the cached promise when authentication fails with AuthCancelledError", async () => {
439+
const cancellationError = new AuthCancelledError(TEST_PROFILE_NAME, "cancelled");
440+
const lockProfileMock = jest.spyOn(AuthHandler, "lockProfile").mockRejectedValueOnce(cancellationError);
441+
442+
await expect(AuthHandler.getOrCreateAuthFlow(TEST_PROFILE_NAME, authOpts)).rejects.toBe(cancellationError);
443+
expect(lockProfileMock).toHaveBeenCalledTimes(1);
444+
expect(AuthHandler.getActiveAuthFlow(TEST_PROFILE_NAME)).toBeUndefined();
445+
lockProfileMock.mockRestore();
446+
});
447+
448+
it("starts a new flow after the previous one resolves", async () => {
449+
const lockProfileMock = jest.spyOn(AuthHandler, "lockProfile").mockClear().mockResolvedValueOnce(true).mockResolvedValueOnce(true);
450+
451+
await expect(AuthHandler.getOrCreateAuthFlow(TEST_PROFILE_NAME, authOpts)).resolves.toBeUndefined();
452+
expect(AuthHandler.getActiveAuthFlow(TEST_PROFILE_NAME)).toBeUndefined();
453+
454+
await expect(AuthHandler.getOrCreateAuthFlow(TEST_PROFILE_NAME, authOpts)).resolves.toBeUndefined();
455+
expect(lockProfileMock).toHaveBeenCalledTimes(2);
456+
lockProfileMock.mockRestore();
457+
});
458+
});
459+
460+
describe("sequential request management", () => {
461+
beforeEach(() => {
462+
(AuthHandler as any).parallelEnabledProfiles.clear();
463+
(AuthHandler as any).sequentialLocks.clear();
464+
});
465+
466+
it("runs sequentially by default until a request completes successfully", async () => {
467+
const order: number[] = [];
468+
let resolveFirst: (() => void) | undefined;
469+
470+
const first = AuthHandler.runSequentialIfEnabled(TEST_PROFILE_NAME, async () => {
471+
order.push(1);
472+
await new Promise<void>((resolve) => {
473+
resolveFirst = resolve;
474+
});
475+
order.push(2);
476+
});
477+
478+
// eslint-disable-next-line @typescript-eslint/require-await
479+
const second = AuthHandler.runSequentialIfEnabled(TEST_PROFILE_NAME, async () => {
480+
order.push(3);
481+
});
482+
483+
await Promise.resolve();
484+
expect(order).toEqual([1]);
485+
486+
resolveFirst?.();
487+
await Promise.all([first, second]);
488+
expect(order).toEqual([1, 2, 3]);
489+
});
490+
491+
it("enables and disables sequential handling for a profile", () => {
492+
AuthHandler.enableSequentialRequests(TEST_PROFILE_NAME);
493+
expect(AuthHandler.areSequentialRequestsEnabled(TEST_PROFILE_NAME)).toBe(true);
494+
495+
AuthHandler.disableSequentialRequests(TEST_PROFILE_NAME);
496+
expect(AuthHandler.areSequentialRequestsEnabled(TEST_PROFILE_NAME)).toBe(false);
400497
});
401-
it("returns false if the user is currently responding to a credential prompt", async () => {
402-
await expect(AuthHandler.shouldHandleAuthError(TEST_PROFILE_NAME)).resolves.toBe(false);
498+
499+
it("does not enforce sequential order when disabled", async () => {
500+
AuthHandler.disableSequentialRequests(TEST_PROFILE_NAME);
501+
502+
const order: number[] = [];
503+
let resolveFirst: (() => void) | undefined;
504+
505+
// eslint-disable-next-line @typescript-eslint/require-await
506+
const first = AuthHandler.runSequentialIfEnabled(TEST_PROFILE_NAME, async () => {
507+
order.push(1);
508+
await new Promise<void>((resolve) => {
509+
resolveFirst = resolve;
510+
});
511+
order.push(2);
512+
});
513+
514+
// eslint-disable-next-line @typescript-eslint/require-await
515+
const second = AuthHandler.runSequentialIfEnabled(TEST_PROFILE_NAME, async () => {
516+
order.push(3);
517+
});
518+
519+
await Promise.resolve();
520+
expect(order).toEqual([1, 3]);
521+
522+
resolveFirst?.();
523+
await Promise.all([first, second]);
524+
expect(order).toEqual([1, 3, 2]);
525+
});
526+
527+
it("releases queued actions once parallel mode is re-enabled", async () => {
528+
const order: string[] = [];
529+
let resolveFirst: (() => void) | undefined;
530+
531+
const first = AuthHandler.runSequentialIfEnabled(TEST_PROFILE_NAME, async () => {
532+
order.push("first-start");
533+
await new Promise<void>((resolve) => {
534+
resolveFirst = resolve;
535+
});
536+
order.push("first-end");
537+
});
538+
539+
await Promise.resolve();
540+
541+
// eslint-disable-next-line @typescript-eslint/require-await
542+
const second = AuthHandler.runSequentialIfEnabled(TEST_PROFILE_NAME, async () => {
543+
order.push("second-run");
544+
});
545+
546+
await Promise.resolve();
547+
expect(order).toEqual(["first-start"]);
548+
549+
AuthHandler.disableSequentialRequests(TEST_PROFILE_NAME);
550+
551+
await Promise.resolve();
552+
await Promise.resolve();
553+
554+
expect(order).toEqual(["first-start", "second-run"]);
555+
556+
resolveFirst?.();
557+
await Promise.all([first, second]);
558+
expect(order).toEqual(["first-start", "second-run", "first-end"]);
403559
});
404560
});
405561

0 commit comments

Comments
 (0)