Merge pull request 'Show the real startupservice response text on a malformed reply' (#297) from issue-277 into version7-13

Reviewed-on: #297

client/src/app/deploy/sections/manual/manual.component.spec.ts

@@ -0,0 +1,90 @@
+import { ManualComponent } from './manual.component'
+
+// validateDeploy() is fire-and-forget (doesn't return its own promise
+// chain), so tests need to wait for it to settle some other way. A
+// macrotask boundary guarantees every already-queued microtask (regardless
+// of how many .then() hops validateDeploy's chain has) has run first.
+const flushPromiseChain = () => new Promise((resolve) => setTimeout(resolve, 0))
+
+/**
+ * validateDeploy() only touches sasService.request, eventService.showInfoModal
+ * and loggerService.log - these stubs cover exactly that surface, not the
+ * full real services (no TestBed/DI needed, same plain-instantiation-with-
+ * stubs precedent as app.service.spec.ts).
+ */
+const buildDeps = () => {
+  const sasService: any = {
+    request: jasmine.createSpy('request')
+  }
+  const eventService: any = {
+    showInfoModal: jasmine.createSpy('showInfoModal')
+  }
+  const loggerService: any = {
+    log: jasmine.createSpy('log')
+  }
+  const deployService: any = {}
+
+  return { sasService, eventService, loggerService, deployService }
+}
+
+const buildManualComponent = (deps: ReturnType<typeof buildDeps>) =>
+  new ManualComponent(
+    deps.sasService,
+    deps.eventService,
+    deps.loggerService,
+    deps.deployService
+  )
+
+describe('ManualComponent - validateDeploy', () => {
+  it('sets validationState to success when saslibs is present on a normal object response', async () => {
+    const deps = buildDeps()
+    deps.sasService.request.and.resolveTo({
+      adapterResponse: { saslibs: { SOME_LIB: ['SOME_TABLE'] } }
+    })
+
+    const component = buildManualComponent(deps)
+    component.validateDeploy()
+    await flushPromiseChain()
+
+    expect(component.validationState).toBe('success')
+    expect(deps.eventService.showInfoModal).not.toHaveBeenCalled()
+  })
+
+  it('sets validationState to error with no modal when saslibs is missing from an otherwise normal object response', async () => {
+    const deps = buildDeps()
+    deps.sasService.request.and.resolveTo({
+      adapterResponse: { SYSSITE: 'SITE1' }
+      // saslibs deliberately omitted
+    })
+
+    const component = buildManualComponent(deps)
+    component.validateDeploy()
+    await flushPromiseChain()
+
+    expect(component.validationState).toBe('error')
+    expect(deps.eventService.showInfoModal).not.toHaveBeenCalled()
+  })
+
+  // Same underlying bug as app.service.spec.ts's equivalent test: a
+  // misconfigured Viya computeTasks deployment returns a raw "Job error"
+  // text body, which the adapter resolves as adapterResponse verbatim.
+  it('sets validationState to error and shows the real response text when adapterResponse is a raw string, not an object', async () => {
+    const deps = buildDeps()
+    const rawJobError = [
+      'Job error',
+      'The Compute service could not execute the task c74a707a-8b88-46a5-8f28-9a7694ad5e13 because the context 340cd3eb-72ae is not reusable. Please provide a reusable context or supply the ID of an existing session in the task request.',
+      'path: /compute/tasks',
+      'correlator: dc1a9fda-8f65-4f32-b4d4-2a70e02179b1;1bcb6a4c-8a16-4053-a790-292544c25a03'
+    ].join('\n')
+    deps.sasService.request.and.resolveTo({ adapterResponse: rawJobError })
+
+    const component = buildManualComponent(deps)
+    component.validateDeploy()
+    await flushPromiseChain()
+
+    expect(component.validationState).toBe('error')
+    expect(deps.eventService.showInfoModal).toHaveBeenCalledTimes(1)
+    const [, message] = deps.eventService.showInfoModal.calls.mostRecent().args
+    expect(message).toContain(rawJobError)
+  })
+})

client/src/app/deploy/sections/manual/manual.component.ts

@@ -13,6 +13,7 @@ import { DeployService } from 'src/app/services/deploy.service'
 import { EventService } from 'src/app/services/event.service'
 import { LoggerService } from 'src/app/services/logger.service'
 import { SasService } from 'src/app/services/sas.service'
+import { getMalformedAdapterResponseMessage } from 'src/app/shared/utils/get-malformed-adapter-response-message'
 
 @Component({
   selector: 'app-manual-deploy',
@@ -316,7 +317,13 @@ export class ManualComponent implements OnInit {
       .then((res: RequestWrapperResponse) => {
         this.loggerService.log(res.adapterResponse)
 
-        if (res.adapterResponse.saslibs) {
+        const malformedMessage = getMalformedAdapterResponseMessage(
+          res.adapterResponse
+        )
+        if (malformedMessage) {
+          this.validationState = 'error'
+          this.eventService.showInfoModal('Error', malformedMessage)
+        } else if (res.adapterResponse.saslibs) {
           this.validationState = 'success'
         } else {
           this.validationState = 'error'

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

@@ -138,4 +138,36 @@ describe('AppService - startup retry', () => {
     expect(deps.sasService.request).toHaveBeenCalledTimes(1)
     expect(deps.eventService.showInfoModal).toHaveBeenCalledTimes(1)
   })
+
+  // Reproduces a misconfigured Viya computeTasks deployment: the Compute
+  // service returns a plain-text "Job error" body instead of JSON: the
+  // adapter can't parse it and resolves with the raw text as adapterResponse
+  // itself (see getMalformedAdapterResponseMessage's own doc comment).
+  // "Globvars, Sasdatasets, Saslibs, XLMaps are not present" would also be
+  // technically true here, but hides the real cause.
+  it('shows the real response text (not the generic missing-props message) when adapterResponse is a raw string, not an object', async () => {
+    const deps = buildDeps()
+    const rawJobError = [
+      'Job error',
+      'The Compute service could not execute the task c74a707a-8b88-46a5-8f28-9a7694ad5e13 because the context 340cd3eb-72ae is not reusable. Please provide a reusable context or supply the ID of an existing session in the task request.',
+      'path: /compute/tasks',
+      'correlator: dc1a9fda-8f65-4f32-b4d4-2a70e02179b1;1bcb6a4c-8a16-4053-a790-292544c25a03'
+    ].join('\n')
+    deps.sasService.request.and.resolveTo({ adapterResponse: rawJobError })
+
+    const appService = buildAppService(deps)
+    ;(appService as any).retryOptions = {
+      wait: () => Promise.resolve(),
+      random: () => 0
+    }
+
+    await appService.startUpData()
+
+    expect(deps.sasService.request).toHaveBeenCalledTimes(1)
+    expect(deps.eventService.showInfoModal).toHaveBeenCalledTimes(1)
+    const [, message] = deps.eventService.showInfoModal.calls.mostRecent().args
+    expect(message).toContain(rawJobError)
+    expect(message).not.toContain('Globvars, Sasdatasets, Saslibs, XLMaps')
+    expect(deps.licenceService.isAppActivated.value).toBeFalse()
+  })
 })

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

@@ -12,6 +12,7 @@ import { AppThemes } from '../models/AppSettings'
 import { RequestWrapperResponse } from '../models/request-wrapper/RequestWrapperResponse'
 import { AppStoreService } from './app-store.service'
 import { retryOnce, RetryOnceOptions } from '../shared/utils/retry-once'
+import { getMalformedAdapterResponseMessage } from '../shared/utils/get-malformed-adapter-response-message'
 
 @Injectable()
 export class AppService {
@@ -94,6 +95,17 @@ export class AppService {
       this.retryOptions
     )
       .then(async (res: RequestWrapperResponse) => {
+        const malformedMessage = getMalformedAdapterResponseMessage(
+          res.adapterResponse
+        )
+        if (malformedMessage) {
+          startupServiceError = true
+          this.eventService.showInfoModal('Error', malformedMessage)
+          this.licenceService.isAppActivated.next(false)
+
+          return
+        }
+
         this.syssite.next([res.adapterResponse.SYSSITE])
 
         let missingProps: string[] = []

client/src/app/shared/utils/get-malformed-adapter-response-message.spec.ts

@@ -0,0 +1,50 @@
+import { getMalformedAdapterResponseMessage } from './get-malformed-adapter-response-message'
+
+describe('getMalformedAdapterResponseMessage', () => {
+  it('returns null for a normal object response', () => {
+    expect(
+      getMalformedAdapterResponseMessage({
+        SYSSITE: 'SITE1',
+        globvars: [{ ISADMIN: false }],
+        sasdatasets: [],
+        saslibs: {},
+        xlmaps: []
+      })
+    ).toBeNull()
+  })
+
+  it("returns null for an object missing individual fields - that stays the caller's missing-props concern", () => {
+    expect(
+      getMalformedAdapterResponseMessage({
+        SYSSITE: 'SITE1'
+        // globvars/sasdatasets/saslibs/xlmaps deliberately omitted
+      })
+    ).toBeNull()
+  })
+
+  it('returns a message containing the raw text verbatim for a raw "Job error" string response', () => {
+    const rawJobError = [
+      'Job error',
+      'The Compute service could not execute the task c74a707a-8b88-46a5-8f28-9a7694ad5e13 because the context 340cd3eb-72ae is not reusable. Please provide a reusable context or supply the ID of an existing session in the task request.',
+      'path: /compute/tasks',
+      'correlator: dc1a9fda-8f65-4f32-b4d4-2a70e02179b1;1bcb6a4c-8a16-4053-a790-292544c25a03'
+    ].join('\n')
+
+    const message = getMalformedAdapterResponseMessage(rawJobError)
+
+    expect(message).not.toBeNull()
+    expect(message).toContain(rawJobError)
+  })
+
+  it('returns a message for null', () => {
+    expect(getMalformedAdapterResponseMessage(null)).not.toBeNull()
+  })
+
+  it('returns a message for undefined', () => {
+    expect(getMalformedAdapterResponseMessage(undefined)).not.toBeNull()
+  })
+
+  it('returns a message for a number', () => {
+    expect(getMalformedAdapterResponseMessage(404)).not.toBeNull()
+  })
+})

client/src/app/shared/utils/get-malformed-adapter-response-message.ts

@@ -0,0 +1,21 @@
+/**
+ * A successful startupservice/deploy-validation response is always a plain
+ * JSON object. When SAS Viya's Compute service can't run the underlying job
+ * (e.g. computeTasks misconfigured), it can return a 202 whose body is a
+ * plain-text "Job error ... path: /compute/tasks ... correlator: ..." block
+ * instead - @sasjs/adapter fails to JSON.parse it, doesn't recognize the
+ * shape as an error either, and resolves with that raw text as-is. Property
+ * access on a string is always undefined, so callers that only check "are
+ * the expected fields present" end up reporting every field as missing
+ * without ever showing the real underlying text. Detecting "not an object
+ * at all" up front lets a caller show that real text instead.
+ */
+export const getMalformedAdapterResponseMessage = (
+  adapterResponse: unknown
+): string | null => {
+  if (adapterResponse !== null && typeof adapterResponse === 'object') {
+    return null
+  }
+
+  return `The startupservice response was not in the expected format - it should be a JSON object, but the following was received instead:\n\n${String(adapterResponse)}`
+}