Merge pull request #518 from sasjs/loginFix

fix: web request and sas9 login

src/FileUploader.ts

@@ -61,24 +61,21 @@ export class FileUploader {
       'Content-Type': 'text/plain'
     }
 
+    // currently only web approach is supported for file upload
+    // therefore log is part of response with debug enabled and must be parsed
     return this.requestClient
       .post(uploadUrl, formData, undefined, 'application/json', headers)
       .then(async (res) => {
-        // for web approach on Viya
         if (
-          this.sasjsConfig.debug &&
-          (this.sasjsConfig.useComputeApi === null ||
-            this.sasjsConfig.useComputeApi === undefined) &&
-          this.sasjsConfig.serverType === ServerType.SasViya
+          this.sasjsConfig.serverType === ServerType.SasViya &&
+          this.sasjsConfig.debug
         ) {
           const jsonResponse = await parseSasViyaDebugResponse(
             res.result as string,
             this.requestClient,
             this.sasjsConfig.serverUrl
           )
-          return typeof jsonResponse === 'string'
-            ? getValidJson(jsonResponse)
-            : jsonResponse
+          return jsonResponse
         }
 
         return typeof res.result === 'string'

src/SAS9ApiClient.ts

@@ -10,9 +10,13 @@ import { isUrl } from './utils'
 export class SAS9ApiClient {
   private requestClient: Sas9RequestClient
 
-  constructor(private serverUrl: string, private jobsPath: string) {
+  constructor(
+    private serverUrl: string,
+    private jobsPath: string,
+    allowInsecureRequests: boolean
+  ) {
     if (serverUrl) isUrl(serverUrl)
-    this.requestClient = new Sas9RequestClient(serverUrl, false)
+    this.requestClient = new Sas9RequestClient(serverUrl, allowInsecureRequests)
   }
 
   /**

src/SASjs.ts

@@ -619,6 +619,11 @@ export default class SASjs {
             authConfig
           )
         } else {
+          if (!config.contextName)
+            config = {
+              ...config,
+              contextName: 'SAS Job Execution compute context'
+            }
           return await this.jesJobExecutor!.execute(
             sasJob,
             data,
@@ -749,7 +754,11 @@ export default class SASjs {
         )
         sasApiClient.debug = this.sasjsConfig.debug
       } else if (this.sasjsConfig.serverType === ServerType.Sas9) {
-        sasApiClient = new SAS9ApiClient(serverUrl, this.jobsPath)
+        sasApiClient = new SAS9ApiClient(
+          serverUrl,
+          this.jobsPath,
+          this.sasjsConfig.allowInsecureRequests
+        )
       }
     } else {
       let sasClientConfig: any = null
@@ -944,7 +953,8 @@ export default class SASjs {
       else
         this.sas9ApiClient = new SAS9ApiClient(
           this.sasjsConfig.serverUrl,
-          this.jobsPath
+          this.jobsPath,
+          this.sasjsConfig.allowInsecureRequests
         )
     }
 
@@ -965,7 +975,8 @@ export default class SASjs {
     this.sas9JobExecutor = new Sas9JobExecutor(
       this.sasjsConfig.serverUrl,
       this.sasjsConfig.serverType!,
-      this.jobsPath
+      this.jobsPath,
+      this.sasjsConfig.allowInsecureRequests
     )
 
     this.computeJobExecutor = new ComputeJobExecutor(

src/SessionManager.ts

@@ -5,10 +5,10 @@ import { prefixMessage } from '@sasjs/utils/error'
 import { RequestClient } from './request/RequestClient'
 
 const MAX_SESSION_COUNT = 1
-const RETRY_LIMIT: number = 3
-let RETRY_COUNT: number = 0
 
 export class SessionManager {
+  private loggedErrors: NoSessionStateError[] = []
+
   constructor(
     private serverUrl: string,
     private contextName: string,
@@ -154,69 +154,75 @@ export class SessionManager {
     session: Session,
     etag: string | null,
     accessToken?: string
-  ) {
+  ): Promise<string> {
     const logger = process.logger || console
 
     let sessionState = session.state
 
     const stateLink = session.links.find((l: any) => l.rel === 'state')
 
-    return new Promise(async (resolve, reject) => {
-      if (
-        sessionState === 'pending' ||
-        sessionState === 'running' ||
-        sessionState === ''
-      ) {
-        if (stateLink) {
-          if (this.debug && !this.printedSessionState.printed) {
-            logger.info('Polling session status...')
+    if (
+      sessionState === 'pending' ||
+      sessionState === 'running' ||
+      sessionState === ''
+    ) {
+      if (stateLink) {
+        if (this.debug && !this.printedSessionState.printed) {
+          logger.info('Polling session status...')
 
-            this.printedSessionState.printed = true
-          }
-
-          const { result: state, responseStatus: responseStatus } =
-            await this.getSessionState(
-              `${this.serverUrl}${stateLink.href}?wait=30`,
-              etag!,
-              accessToken
-            ).catch((err) => {
-              throw prefixMessage(err, 'Error while getting session state.')
-            })
-
-          sessionState = state.trim()
-
-          if (this.debug && this.printedSessionState.state !== sessionState) {
-            logger.info(`Current session state is '${sessionState}'`)
-
-            this.printedSessionState.state = sessionState
-            this.printedSessionState.printed = false
-          }
-
-          // There is an internal error present in SAS Viya 3.5
-          // Retry to wait for a session status in such case of SAS internal error
-          if (!sessionState) {
-            if (RETRY_COUNT < RETRY_LIMIT) {
-              RETRY_COUNT++
-
-              resolve(this.waitForSession(session, etag, accessToken))
-            } else {
-              reject(
-                new NoSessionStateError(
-                  responseStatus,
-                  this.serverUrl + stateLink.href,
-                  session.links.find((l: any) => l.rel === 'log')
-                    ?.href as string
-                )
-              )
-            }
-          }
-
-          resolve(sessionState)
+          this.printedSessionState.printed = true
         }
+
+        const { result: state, responseStatus: responseStatus } =
+          await this.getSessionState(
+            `${this.serverUrl}${stateLink.href}?wait=30`,
+            etag!,
+            accessToken
+          ).catch((err) => {
+            throw prefixMessage(err, 'Error while getting session state.')
+          })
+
+        sessionState = state.trim()
+
+        if (this.debug && this.printedSessionState.state !== sessionState) {
+          logger.info(`Current session state is '${sessionState}'`)
+
+          this.printedSessionState.state = sessionState
+          this.printedSessionState.printed = false
+        }
+
+        if (!sessionState) {
+          const stateError = new NoSessionStateError(
+            responseStatus,
+            this.serverUrl + stateLink.href,
+            session.links.find((l: any) => l.rel === 'log')?.href as string
+          )
+
+          if (
+            !this.loggedErrors.find(
+              (err: NoSessionStateError) =>
+                err.serverResponseStatus === stateError.serverResponseStatus
+            )
+          ) {
+            this.loggedErrors.push(stateError)
+
+            logger.info(stateError.message)
+          }
+
+          return await this.waitForSession(session, etag, accessToken)
+        }
+
+        this.loggedErrors = []
+
+        return sessionState
       } else {
-        resolve(sessionState)
+        throw 'Error while getting session state link.'
       }
-    })
+    } else {
+      this.loggedErrors = []
+
+      return sessionState
+    }
   }
 
   private async getSessionState(

src/auth/AuthManager.ts

@@ -124,7 +124,8 @@ export class AuthManager {
 
     if (!isLoggedIn) {
       //We will logout to make sure cookies are removed and login form is presented
-      this.logOut()
+      //Residue can happen in case of session expiration
+      await this.logOut()
 
       const { result: formResponse } = await this.requestClient.get<string>(
         this.loginUrl.replace('.do', ''),

src/job-execution/Sas9JobExecutor.ts

@@ -16,10 +16,11 @@ export class Sas9JobExecutor extends BaseJobExecutor {
   constructor(
     serverUrl: string,
     serverType: ServerType,
-    private jobsPath: string
+    private jobsPath: string,
+    allowInsecureRequests: boolean
   ) {
     super(serverUrl, serverType)
-    this.requestClient = new Sas9RequestClient(serverUrl, false)
+    this.requestClient = new Sas9RequestClient(serverUrl, allowInsecureRequests)
   }
 
   async execute(sasJob: string, data: any, config: any) {

src/job-execution/WebJobExecutor.ts

@@ -2,7 +2,8 @@ import { ServerType } from '@sasjs/utils/types'
 import {
   ErrorResponse,
   JobExecutionError,
-  LoginRequiredError
+  LoginRequiredError,
+  WeboutResponseError
 } from '../types/errors'
 import { generateFileUploadForm } from '../file/generateFileUploadForm'
 import { generateTableUploadForm } from '../file/generateTableUploadForm'
@@ -54,7 +55,21 @@ export class WebJobExecutor extends BaseJobExecutor {
 
       apiUrl += jobUri.length > 0 ? '&_job=' + jobUri : ''
 
-      apiUrl += config.contextName ? `&_contextname=${config.contextName}` : ''
+      if (jobUri.length > 0) {
+        apiUrl += '&_job=' + jobUri
+        /**
+         * Using both _job and _program parameters will cause a conflict in the JES web app, as it’s not clear whether or not the server should make the extra fetch for the job uri.
+         * To handle this, we add the extra underscore and recreate the _program variable in the SAS side of the SASjs adapter so it remains available for backend developers.
+         */
+        apiUrl = apiUrl.replace('_program=', '__program=')
+      }
+
+      // if context name exists and is not blank string
+      // then add _contextname variable in apiUrl
+      apiUrl +=
+        config.contextName && !/\s/.test(config.contextName)
+          ? `&_contextname=${config.contextName}`
+          : ''
     }
 
     let requestParams = {
@@ -97,10 +112,10 @@ export class WebJobExecutor extends BaseJobExecutor {
 
     const requestPromise = new Promise((resolve, reject) => {
       this.requestClient!.post(apiUrl, formData, undefined)
-        .then(async (res) => {
+        .then(async (res: any) => {
           if (this.serverType === ServerType.SasViya && config.debug) {
             const jsonResponse = await parseSasViyaDebugResponse(
-              res.result as string,
+              res.result,
               this.requestClient,
               this.serverUrl
             )
@@ -108,19 +123,16 @@ export class WebJobExecutor extends BaseJobExecutor {
             resolve(jsonResponse)
           }
           if (this.serverType === ServerType.Sas9 && config.debug) {
-            const jsonResponse = parseWeboutResponse(res.result as string)
-            if (jsonResponse === '') {
-              throw new Error(
-                'Valid JSON could not be extracted from response.'
-              )
-            }
+            let jsonResponse = res.result
+            if (typeof res.result === 'string')
+              jsonResponse = parseWeboutResponse(res.result, apiUrl)
 
             getValidJson(jsonResponse)
             this.appendRequest(res, sasJob, config.debug)
             resolve(res.result)
           }
-          getValidJson(res.result as string)
           this.appendRequest(res, sasJob, config.debug)
+          getValidJson(res.result as string)
           resolve(res.result)
         })
         .catch(async (e: Error) => {

src/request/RequestClient.ts

@@ -429,13 +429,7 @@ export class RequestClient implements HttpClient {
       }
     } catch {
       try {
-        const weboutResponse = parseWeboutResponse(response.data)
-        if (weboutResponse === '') {
-          throw new Error('Valid JSON could not be extracted from response.')
-        }
-
-        const jsonResponse = getValidJson(weboutResponse)
-        parsedResponse = jsonResponse
+        parsedResponse = JSON.parse(parseWeboutResponse(response.data))
       } catch {
         parsedResponse = response.data
       }

src/test/SessionManager.spec.ts

@@ -3,6 +3,8 @@ import { RequestClient } from '../request/RequestClient'
 import { NoSessionStateError } from '../types/errors'
 import * as dotenv from 'dotenv'
 import axios from 'axios'
+import { Logger, LogLevel } from '@sasjs/utils'
+import { Session } from '../types'
 
 jest.mock('axios')
 const mockedAxios = axios as jest.Mocked<typeof axios>
@@ -47,36 +49,91 @@ describe('SessionManager', () => {
   })
 
   describe('waitForSession', () => {
+    const session: Session = {
+      id: 'id',
+      state: '',
+      links: [{ rel: 'state', href: '', uri: '', type: '', method: 'GET' }],
+      attributes: {
+        sessionInactiveTimeout: 0
+      },
+      creationTimeStamp: ''
+    }
+
+    beforeEach(() => {
+      ;(process as any).logger = new Logger(LogLevel.Off)
+    })
+
     it('should reject with NoSessionStateError if SAS server did not provide session state', async () => {
-      const responseStatus = 304
+      let requestAttempt = 0
+      const requestAttemptLimit = 10
+      const sessionState = 'idle'
+
+      mockedAxios.get.mockImplementation(() => {
+        requestAttempt += 1
+
+        if (requestAttempt >= requestAttemptLimit) {
+          return Promise.resolve({ data: sessionState, status: 200 })
+        }
+
+        return Promise.resolve({ data: '', status: 304 })
+      })
+
+      jest.spyOn((process as any).logger, 'info')
+
+      sessionManager.debug = true
+
+      await expect(
+        sessionManager['waitForSession'](session, null, 'access_token')
+      ).resolves.toEqual(sessionState)
+
+      expect(mockedAxios.get).toHaveBeenCalledTimes(requestAttemptLimit)
+      expect((process as any).logger.info).toHaveBeenCalledTimes(3)
+      expect((process as any).logger.info).toHaveBeenNthCalledWith(
+        1,
+        'Polling session status...'
+      )
+      expect((process as any).logger.info).toHaveBeenNthCalledWith(
+        2,
+        `Could not get session state. Server responded with 304 whilst checking state: ${process.env.SERVER_URL}`
+      )
+      expect((process as any).logger.info).toHaveBeenNthCalledWith(
+        3,
+        `Current session state is '${sessionState}'`
+      )
+    })
+
+    it('should throw an error if there is no session link', async () => {
+      const customSession = JSON.parse(JSON.stringify(session))
+      customSession.links = []
 
       mockedAxios.get.mockImplementation(() =>
-        Promise.resolve({ data: '', status: responseStatus })
+        Promise.resolve({ data: customSession.state, status: 200 })
       )
 
       await expect(
-        sessionManager['waitForSession'](
-          {
-            id: 'id',
-            state: '',
-            links: [
-              { rel: 'state', href: '', uri: '', type: '', method: 'GET' }
-            ],
-            attributes: {
-              sessionInactiveTimeout: 0
-            },
-            creationTimeStamp: ''
-          },
-          null,
-          'access_token'
-        )
-      ).rejects.toEqual(
-        new NoSessionStateError(
-          responseStatus,
-          process.env.SERVER_URL as string,
-          'logUrl'
-        )
+        sessionManager['waitForSession'](customSession, null, 'access_token')
+      ).rejects.toContain('Error while getting session state link.')
+    })
+
+    it('should throw an error if could not get session state', async () => {
+      mockedAxios.get.mockImplementation(() => Promise.reject('Mocked error'))
+
+      await expect(
+        sessionManager['waitForSession'](session, null, 'access_token')
+      ).rejects.toContain('Error while getting session state.')
+    })
+
+    it('should return session state', async () => {
+      const customSession = JSON.parse(JSON.stringify(session))
+      customSession.state = 'completed'
+
+      mockedAxios.get.mockImplementation(() =>
+        Promise.resolve({ data: customSession.state, status: 200 })
       )
+
+      await expect(
+        sessionManager['waitForSession'](customSession, null, 'access_token')
+      ).resolves.toEqual(customSession.state)
     })
   })
 })

src/test/utils/getValidJson.spec.ts

@@ -1,4 +1,5 @@
 import { getValidJson } from '../../utils'
+import { JsonParseArrayError, InvalidJsonError } from '../../types/errors'
 
 describe('jsonValidator', () => {
   it('should not throw an error with a valid json', () => {
@@ -19,23 +20,17 @@ describe('jsonValidator', () => {
 
   it('should throw an error with an invalid json', () => {
     const json = `{\"test\":\"test\"\"test2\":\"test\"}`
-    let errorThrown = false
-    try {
+    const test = () => {
       getValidJson(json)
-    } catch (error) {
-      errorThrown = true
     }
-    expect(errorThrown).toBe(true)
+    expect(test).toThrowError(InvalidJsonError)
   })
 
   it('should throw an error when an array is passed', () => {
     const array = ['hello', 'world']
-    let errorThrown = false
-    try {
+    const test = () => {
       getValidJson(array)
-    } catch (error) {
-      errorThrown = true
     }
-    expect(errorThrown).toBe(true)
+    expect(test).toThrow(JsonParseArrayError)
   })
 })

src/types/errors/InvalidJsonError.ts

@@ -0,0 +1,7 @@
+export class InvalidJsonError extends Error {
+  constructor() {
+    super('Error: invalid Json string')
+    this.name = 'InvalidJsonError'
+    Object.setPrototypeOf(this, InvalidJsonError.prototype)
+  }
+}

src/types/errors/JsonParseArrayError.ts

@@ -0,0 +1,7 @@
+export class JsonParseArrayError extends Error {
+  constructor() {
+    super('Can not parse array object to json.')
+    this.name = 'JsonParseArrayError'
+    Object.setPrototypeOf(this, JsonParseArrayError.prototype)
+  }
+}

src/types/errors/WeboutResponseError.ts

@@ -0,0 +1,7 @@
+export class WeboutResponseError extends Error {
+  constructor(public url: string) {
+    super(`Error: error while parsing response from ${url}`)
+    this.name = 'WeboutResponseError'
+    Object.setPrototypeOf(this, WeboutResponseError.prototype)
+  }
+}

src/types/errors/index.ts

@@ -8,3 +8,6 @@ export * from './NotFoundError'
 export * from './ErrorResponse'
 export * from './NoSessionStateError'
 export * from './RootFolderNotFoundError'
+export * from './JsonParseArrayError'
+export * from './WeboutResponseError'
+export * from './InvalidJsonError'

src/utils/getValidJson.ts

@@ -1,16 +1,18 @@
+import { JsonParseArrayError, InvalidJsonError } from '../types/errors'
+
 /**
  * if string passed then parse the string to json else if throw error for all other types unless it is not a valid json object.
  * @param str - string to check.
  */
 export const getValidJson = (str: string | object) => {
   try {
-    if (Array.isArray(str)) {
-      throw new Error('Can not parse array object to json.')
-    }
+    if (Array.isArray(str)) throw new JsonParseArrayError()
+
     if (typeof str === 'object') return str
 
     return JSON.parse(str)
   } catch (e) {
-    throw new Error('Invalid JSON response.')
+    if (e instanceof JsonParseArrayError) throw e
+    throw new InvalidJsonError()
   }
 }

src/utils/parseViyaDebugResponse.ts

@@ -1,4 +1,5 @@
 import { RequestClient } from '../request/RequestClient'
+import { getValidJson } from '../utils'
 
 /**
  * When querying a Viya job using the Web approach (as opposed to using the APIs) with _DEBUG enabled,
@@ -25,5 +26,5 @@ export const parseSasViyaDebugResponse = async (
 
   return requestClient
     .get(serverUrl + jsonUrl, undefined)
-    .then((res) => res.result)
+    .then((res: any) => getValidJson(res.result))
 }

src/utils/parseWeboutResponse.ts

@@ -1,4 +1,6 @@
-export const parseWeboutResponse = (response: string) => {
+import { WeboutResponseError } from '../types/errors'
+
+export const parseWeboutResponse = (response: string, url?: string) => {
   let sasResponse = ''
 
   if (response.includes('>>weboutBEGIN<<')) {
@@ -7,6 +9,7 @@ export const parseWeboutResponse = (response: string) => {
         .split('>>weboutBEGIN<<')[1]
         .split('>>weboutEND<<')[0]
     } catch (e) {
+      if (url) throw new WeboutResponseError(url)
       sasResponse = ''
       console.error(e)
     }