Merge pull request 'fix(startup): retry startupservice while the session is still loading' (#547) from fix/startup-readiness into release/7.23

Reviewed-on: #547

client/src/app/services/app.service.spec.ts

@@ -120,12 +120,46 @@ describe('AppService - startup retry', () => {
     expect(deps.licenceService.isAppActivated.value).toBeFalse()
   })
 
-  it('does not retry a response that arrives but is missing required properties', async () => {
+  // A cold SAS session can answer startupservice before the libraries are
+  // assigned, and come back without the members. That first answer is not a
+  // verdict: the request is retried for a few seconds before the dialog.
+  it('retries a response that is missing required properties, and succeeds once they arrive', async () => {
+    const deps = buildDeps()
+    deps.sasService.request.and.returnValues(
+      Promise.resolve({
+        adapterResponse: {
+          SYSSITE: 'SITE1',
+          // globvars deliberately omitted
+          sasdatasets: [],
+          saslibs: {},
+          xlmaps: []
+        }
+      }),
+      Promise.resolve(validStartupResponse())
+    )
+
+    const appService = buildAppService(deps)
+    ;(appService as any).retryOptions = {
+      wait: () => Promise.resolve(),
+      random: () => 0
+    }
+    ;(appService as any).startupRetryOptions = {
+      wait: () => Promise.resolve(),
+      delayMs: () => 0
+    }
+
+    await appService.startUpData()
+
+    expect(deps.sasService.request).toHaveBeenCalledTimes(2)
+    expect(deps.eventService.showInfoModal).not.toHaveBeenCalled()
+    expect(deps.eventService.startupDataLoaded).toHaveBeenCalled()
+  })
+
+  it('gives up after the last attempt with one dialog, naming what was missing', async () => {
     const deps = buildDeps()
     deps.sasService.request.and.resolveTo({
       adapterResponse: {
         SYSSITE: 'SITE1',
-        // globvars deliberately omitted
         sasdatasets: [],
         saslibs: {},
         xlmaps: []
@@ -137,11 +171,47 @@ describe('AppService - startup retry', () => {
       wait: () => Promise.resolve(),
       random: () => 0
     }
+    ;(appService as any).startupRetryOptions = {
+      wait: () => Promise.resolve(),
+      delayMs: () => 0,
+      maxAttempts: 3
+    }
 
     await appService.startUpData()
 
-    expect(deps.sasService.request).toHaveBeenCalledTimes(1)
+    expect(deps.sasService.request).toHaveBeenCalledTimes(3)
     expect(deps.eventService.showInfoModal).toHaveBeenCalledTimes(1)
+    const message = deps.eventService.showInfoModal.calls.mostRecent().args[1]
+    expect(message).toContain('Globvars')
+    expect(deps.licenceService.isAppActivated.value).toBeFalse()
+  })
+
+  it('retries an abort body too, and quotes its message when it never clears', async () => {
+    const deps = buildDeps()
+    deps.sasService.request.and.resolveTo({
+      adapterResponse: {
+        SYSSITE: 'SITE1',
+        sasjsAbort: [{ MSG: 'Problem accessing DCLIB.mpe_users table' }]
+      }
+    })
+
+    const appService = buildAppService(deps)
+    ;(appService as any).retryOptions = {
+      wait: () => Promise.resolve(),
+      random: () => 0
+    }
+    ;(appService as any).startupRetryOptions = {
+      wait: () => Promise.resolve(),
+      delayMs: () => 0,
+      maxAttempts: 2
+    }
+
+    await appService.startUpData()
+
+    expect(deps.sasService.request).toHaveBeenCalledTimes(2)
+    expect(deps.eventService.showInfoModal).toHaveBeenCalledTimes(1)
+    const message = deps.eventService.showInfoModal.calls.mostRecent().args[1]
+    expect(message).toContain('Problem accessing DCLIB.mpe_users table')
   })
 
   // Reproduces a misconfigured Viya computeTasks deployment: the Compute

client/src/app/services/app.service.ts

@@ -13,6 +13,7 @@ import { RequestWrapperResponse } from '../models/request-wrapper/RequestWrapper
 import { AppStoreService } from './app-store.service'
 import { StartupCheckService } from './startup-check.service'
 import { retryOnce, RetryOnceOptions } from '../shared/utils/retry-once'
+import { retryUntil, RetryUntilOptions } from '../shared/utils/retry-until'
 import { getMalformedAdapterResponseMessage } from '../shared/utils/get-malformed-adapter-response-message'
 
 @Injectable()
@@ -22,6 +23,12 @@ export class AppService {
   // Overridable in tests to keep the retry pause instant — see
   // retry-once.spec.ts and app.service.spec.ts.
   private retryOptions: RetryOnceOptions = {}
+  // The startupservice can answer before the session is ready (a cold SAS
+  // session, a library not yet assigned) and come back without Globvars,
+  // Sasdatasets, Saslibs and XLMaps. That first answer is not a verdict, so it
+  // is retried for a few seconds before the error dialog appears; overridable
+  // in tests like retryOptions.
+  private startupRetryOptions: RetryUntilOptions = {}
 
   constructor(
     private licenceService: LicenceService,
@@ -86,15 +93,61 @@ export class AppService {
     }
   }
 
+  /**
+   * The members the rest of the app needs before it can render - Globvars
+   * carries the user's admin group and timezone, Sasdatasets/Saslibs the tree,
+   * XLMaps the mapping list. Which of them are absent is what the error dialog
+   * names.
+   */
+  private missingStartupProps(res: RequestWrapperResponse): string[] {
+    const missing: string[] = []
+    const response: any = res?.adapterResponse
+
+    if (!response || typeof response !== 'object') {
+      return ['Globvars', 'Sasdatasets', 'Saslibs', 'XLMaps']
+    }
+
+    if (!response.globvars || !response.globvars[0]) missing.push('Globvars')
+    if (!response.sasdatasets) missing.push('Sasdatasets')
+    if (!response.saslibs) missing.push('Saslibs')
+    if (!response.xlmaps) missing.push('XLMaps')
+
+    return missing
+  }
+
+  /**
+   * True when the answer is a verdict - everything the app needs is present,
+   * or the response is not something a retry can fix.
+   */
+  private startupIsComplete(res: RequestWrapperResponse): boolean {
+    const response: any = res?.adapterResponse
+
+    // The adapter passes a plain-text job error through as the response itself
+    // (a misconfigured compute context, say). Retrying that just delays the
+    // real message, so it counts as a verdict.
+    if (!response || typeof response !== 'object') return true
+
+    // An abort body is a valid response with none of the members - it is the
+    // transient case the retry exists for, so it is never a verdict.
+    if (response.sasjsAbort) return false
+
+    return this.missingStartupProps(res).length === 0
+  }
+
   public async startUpData() {
     let startupServiceError = false
 
     // Retry once on failure — multiple concurrent DC instances (e.g. VA
     // multi-page reports) can cause this request to time out even though a
     // second attempt succeeds. See STARTUP_RETRY_PLAN.md.
-    await retryOnce(
-      () => this.sasService.request('public/startupservice', null),
-      this.retryOptions
+    await retryUntil(
+      () =>
+        retryOnce(
+          () => this.sasService.request('public/startupservice', null),
+          this.retryOptions
+        ),
+      (res: RequestWrapperResponse) => this.startupIsComplete(res),
+      this.startupRetryOptions
     )
       .then(async (res: RequestWrapperResponse) => {
         const malformedMessage = getMalformedAdapterResponseMessage(
@@ -119,22 +172,15 @@ export class AppService {
 
         this.syssite.next([res.adapterResponse.SYSSITE])
 
-        let missingProps: string[] = []
-
-        if (
-          !res.adapterResponse.globvars ||
-          (res.adapterResponse.globvars && !res.adapterResponse.globvars[0])
-        )
-          missingProps.push('Globvars')
-        if (!res.adapterResponse.sasdatasets) missingProps.push('Sasdatasets')
-        if (!res.adapterResponse.saslibs) missingProps.push('Saslibs')
-        if (!res.adapterResponse.xlmaps) missingProps.push('XLMaps')
+        const missingProps = this.missingStartupProps(res)
 
         if (missingProps.length > 0) {
           startupServiceError = true
+          const abortMessage = res.adapterResponse?.sasjsAbort?.[0]?.MSG
           console.error(
             'startUpData: startupservice missing properties:',
-            missingProps.join(', ')
+            missingProps.join(', '),
+            abortMessage ?? ''
           )
           this.startupCheckService.updateStep(
             2,
@@ -143,7 +189,8 @@ export class AppService {
           )
           this.eventService.showInfoModal(
             'Error',
-            `${missingProps.join(', ')} are not present in the startupservice`,
+            `${missingProps.join(', ')} are not present in the startupservice` +
+              (abortMessage ? ` (${abortMessage})` : ''),
             'public/startupservice'
           )
           this.licenceService.isAppActivated.next(false)

client/src/app/shared/utils/retry-until.spec.ts

@@ -0,0 +1,54 @@
+import { retryUntil, RetryUntilOptions } from './retry-until'
+
+describe('retryUntil', () => {
+  const fast = (): RetryUntilOptions => ({
+    wait: () => Promise.resolve(),
+    delayMs: () => 0
+  })
+
+  it('returns the first answer when it is already ready', async () => {
+    const requestFn = jasmine.createSpy('requestFn').and.resolveTo('ready')
+
+    const result = await retryUntil(requestFn, (v) => v === 'ready', fast())
+
+    expect(result).toBe('ready')
+    expect(requestFn).toHaveBeenCalledTimes(1)
+  })
+
+  it('retries while the answer is not ready, and returns the ready one', async () => {
+    const requestFn = jasmine
+      .createSpy('requestFn')
+      .and.returnValues(
+        Promise.resolve('empty'),
+        Promise.resolve('empty'),
+        Promise.resolve('ready')
+      )
+
+    const result = await retryUntil(requestFn, (v) => v === 'ready', fast())
+
+    expect(result).toBe('ready')
+    expect(requestFn).toHaveBeenCalledTimes(3)
+  })
+
+  it('returns the last answer when it never becomes ready', async () => {
+    const requestFn = jasmine.createSpy('requestFn').and.resolveTo('empty')
+
+    const result = await retryUntil(requestFn, (v) => v === 'ready', {
+      ...fast(),
+      maxAttempts: 3
+    })
+
+    expect(result).toBe('empty')
+    expect(requestFn).toHaveBeenCalledTimes(3)
+  })
+
+  it('propagates a rejection, leaving the caller its own retry for the transport', async () => {
+    const requestFn = jasmine
+      .createSpy('requestFn')
+      .and.returnValue(Promise.reject('timeout'))
+
+    await expectAsync(
+      retryUntil(requestFn, () => true, fast())
+    ).toBeRejectedWith('timeout')
+  })
+})

client/src/app/shared/utils/retry-until.ts

@@ -0,0 +1,44 @@
+export interface RetryUntilOptions {
+  /** How many times requestFn may be called. Default 5. */
+  maxAttempts?: number
+  /** Delay before the next attempt, given the attempt that just finished (1-based). */
+  delayMs?: (attempt: number) => number
+  /** Injectable for tests. Default setTimeout-based wait. */
+  wait?: (ms: number) => Promise<void>
+}
+
+const defaultWait = (ms: number): Promise<void> =>
+  new Promise((resolve) => setTimeout(resolve, ms))
+
+/** One second after the first attempt, then two, three, five, five. */
+const defaultDelayMs = (attempt: number): number =>
+  Math.min(1000 * attempt, 5000)
+
+/**
+ * Calls requestFn until isReady accepts the answer, then returns it.
+ *
+ * For a service that can answer before the thing it reports on exists - a cold
+ * SAS session, a library that has not been assigned yet - a first response is
+ * not a verdict, and an immediate error dialog makes the retry the user's job.
+ * The last answer is returned whether or not it was ready, so the caller keeps
+ * a single place to report failure.
+ */
+export const retryUntil = async <T>(
+  requestFn: () => Promise<T>,
+  isReady: (value: T) => boolean,
+  options?: RetryUntilOptions
+): Promise<T> => {
+  const maxAttempts = options?.maxAttempts ?? 5
+  const delayMs = options?.delayMs ?? defaultDelayMs
+  const wait = options?.wait ?? defaultWait
+
+  let value = await requestFn()
+
+  for (let attempt = 1; attempt < maxAttempts; attempt++) {
+    if (isReady(value)) return value
+    await wait(delayMs(attempt))
+    value = await requestFn()
+  }
+
+  return value
+}