From 8dd4ab8cec564ae1b86d9323ee5c67fdfad4a6c3 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Thu, 17 Aug 2023 13:29:27 +0300 Subject: [PATCH 1/3] fix(verbose-mode): fixed handling axios errors --- src/SASjs.ts | 6 +- src/api/viya/executeScript.ts | 2 +- src/request/RequestClient.ts | 80 +++++++- src/test/RequestClient.spec.ts | 353 +++++++++++++++++++++++++-------- 4 files changed, 345 insertions(+), 96 deletions(-) diff --git a/src/SASjs.ts b/src/SASjs.ts index 1320f5b..d88ee4c 100644 --- a/src/SASjs.ts +++ b/src/SASjs.ts @@ -34,7 +34,7 @@ import { Sas9JobExecutor, FileUploader } from './job-execution' -import { AxiosResponse } from 'axios' +import { AxiosResponse, AxiosError } from 'axios' interface ExecuteScriptParams { linesOfCode: string[] @@ -1170,8 +1170,8 @@ export default class SASjs { * @param errorCallBack - function that should be triggered on every HTTP response with the status different from 2**. */ public enableVerboseMode( - successCallBack?: (response: AxiosResponse) => AxiosResponse, - errorCallBack?: (response: AxiosResponse) => AxiosResponse + successCallBack?: (response: AxiosResponse | AxiosError) => AxiosResponse, + errorCallBack?: (response: AxiosResponse | AxiosError) => AxiosResponse ) { this.requestClient?.enableVerboseMode(successCallBack, errorCallBack) } diff --git a/src/api/viya/executeScript.ts b/src/api/viya/executeScript.ts index af08394..502a2b1 100644 --- a/src/api/viya/executeScript.ts +++ b/src/api/viya/executeScript.ts @@ -78,7 +78,7 @@ export async function executeScript( const logger = process.logger || console logger.info( - `Triggered '${relativeJobPath}' with PID ${ + `Triggering '${relativeJobPath}' with PID ${ jobIdVariable.value } at ${timestampToYYYYMMDDHHMMSS()}` ) diff --git a/src/request/RequestClient.ts b/src/request/RequestClient.ts index d0fcf4c..d2efa61 100644 --- a/src/request/RequestClient.ts +++ b/src/request/RequestClient.ts @@ -1,4 +1,10 @@ -import { AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios' +import { + AxiosError, + AxiosInstance, + AxiosRequestConfig, + AxiosResponse +} from 'axios' +import axios from 'axios' import * as https from 'https' import { CsrfToken } from '..' import { isAuthorizeFormRequired, isLogInRequired } from '../auth' @@ -65,6 +71,7 @@ export class RequestClient implements HttpClient { this.csrfToken = { headerName: '', value: '' } this.fileUploadCsrfToken = { headerName: '', value: '' } } + public clearLocalStorageTokens() { localStorage.setItem('accessToken', '') localStorage.setItem('refreshToken', '') @@ -406,10 +413,73 @@ export class RequestClient implements HttpClient { return bodyLines.join('\n') } - private defaultInterceptionCallBack = (response: AxiosResponse) => { - const { status, config, request, data: resData } = response + private defaultInterceptionCallBack = ( + axiosResponse: AxiosResponse | AxiosError + ) => { + // Message indicating absent value. + const noValueMessage = 'Not provided' + + // Fallback request object that can be safely used to form request summary. + type FallbackRequest = { _header?: string; res: { rawHeaders: string[] } } + // _header is not present in responses with status 1** + // rawHeaders are not present in responses with status 1** + let fallbackRequest: FallbackRequest = { + _header: `${noValueMessage}\n`, + res: { rawHeaders: [noValueMessage] } + } + + // Fallback response object that can be safely used to form response summary. + type FallbackResponse = { + status?: number | string + request?: FallbackRequest + config: { data?: string } + data?: unknown + } + let fallbackResponse: FallbackResponse = axiosResponse + + if (axios.isAxiosError(axiosResponse)) { + const { response, request, config } = axiosResponse + + // Try to use axiosResponse.response to form response summary. + if (response) { + fallbackResponse = response + } else { + // Try to use axiosResponse.request to form request summary. + if (request) { + const { _header, _currentRequest } = request + + // Try to use axiosResponse.request._header to form request summary. + if (_header) { + fallbackRequest._header = _header + } + // Try to use axiosResponse.request._currentRequest._header to form request summary. + else if (_currentRequest && _currentRequest._header) { + fallbackRequest._header = _currentRequest._header + } + + const { res } = request + + // Try to use axiosResponse.request.res.rawHeaders to form request summary. + if (res && res.rawHeaders) { + fallbackRequest.res.rawHeaders = res.rawHeaders + } + } + + // Fallback config that can be safely used to form response summary. + const fallbackConfig = { data: noValueMessage } + + fallbackResponse = { + status: noValueMessage, + request: fallbackRequest, + config: config || fallbackConfig, + data: noValueMessage + } + } + } + + const { status, config, request, data: resData } = fallbackResponse const { data: reqData } = config - const { _header: reqHeaders, res } = request + const { _header: reqHeaders, res } = request || fallbackRequest const { rawHeaders } = res // Converts an array of strings into a single string with the following format: @@ -439,7 +509,7 @@ HTTP Response (first 50 lines): ${resHeaders}${parsedResBody ? `\n\n${parsedResBody}` : ''} `) - return response + return axiosResponse } /** diff --git a/src/test/RequestClient.spec.ts b/src/test/RequestClient.spec.ts index d592797..cbdbf1d 100644 --- a/src/test/RequestClient.spec.ts +++ b/src/test/RequestClient.spec.ts @@ -5,6 +5,7 @@ import { app, mockedAuthResponse } from './SAS_server_app' import { ServerType } from '@sasjs/utils/types' import SASjs from '../SASjs' import * as axiosModules from '../utils/createAxiosInstance' +import axios from 'axios' import { LoginRequiredError, AuthorizeError, @@ -14,7 +15,7 @@ import { } from '../types' import { RequestClient } from '../request/RequestClient' import { getTokenRequestErrorPrefixResponse } from '../auth/getTokenRequestErrorPrefix' -import { AxiosResponse } from 'axios' +import { AxiosResponse, AxiosError } from 'axios' import { Logger, LogLevel } from '@sasjs/utils/logger' import * as UtilsModule from 'util' @@ -26,7 +27,7 @@ jest axiosActual.create({ baseURL, httpsAgent }) ) -const PORT = 8000 +const PORT = 8015 const SERVER_URL = `https://localhost:${PORT}/` describe('RequestClient', () => { @@ -75,88 +76,7 @@ describe('RequestClient', () => { }) describe('defaultInterceptionCallBack', () => { - beforeAll(() => { - ;(process as any).logger = new Logger(LogLevel.Off) - }) - - it('should log parsed response', () => { - jest.spyOn((process as any).logger, 'info') - - const status = 200 - const reqData = `{ - name: 'test_job', - description: 'Powered by SASjs', - code: ['test_code'], - variables: { - SYS_JES_JOB_URI: '', - _program: '/Public/sasjs/jobs/jobs/test_job' - }, - arguments: { - _contextName: 'SAS Job Execution compute context', - _OMITJSONLISTING: true, - _OMITJSONLOG: true, - _OMITSESSIONRESULTS: true, - _OMITTEXTLISTING: true, - _OMITTEXTLOG: true - } - }` - const resData = { - id: 'id_string', - name: 'name_string', - uri: 'uri_string', - createdBy: 'createdBy_string', - code: 'TEST CODE', - links: [ - { - method: 'method_string', - rel: 'state', - href: 'state_href_string', - uri: 'uri_string', - type: 'type_string' - }, - { - method: 'method_string', - rel: 'state', - href: 'state_href_string', - uri: 'uri_string', - type: 'type_string' - }, - { - method: 'method_string', - rel: 'state', - href: 'state_href_string', - uri: 'uri_string', - type: 'type_string' - }, - { - method: 'method_string', - rel: 'state', - href: 'state_href_string', - uri: 'uri_string', - type: 'type_string' - }, - { - method: 'method_string', - rel: 'state', - href: 'state_href_string', - uri: 'uri_string', - type: 'type_string' - }, - { - method: 'method_string', - rel: 'self', - href: 'self_href_string', - uri: 'uri_string', - type: 'type_string' - } - ], - results: { '_webout.json': '_webout.json_string' }, - logStatistics: { - lineCount: 1, - modifiedTimeStamp: 'modifiedTimeStamp_string' - } - } - const reqHeaders = `POST https://sas.server.com/compute/sessions/session_id/jobs HTTP/1.1 + const reqHeaders = `POST https://sas.server.com/compute/sessions/session_id/jobs HTTP/1.1 Accept: application/json Content-Type: application/json User-Agent: axios/0.27.2 @@ -164,7 +84,126 @@ Content-Length: 334 host: sas.server.io Connection: close ` - const resHeaders = ['content-type', 'application/json'] + const reqData = `{ + name: 'test_job', + description: 'Powered by SASjs', + code: ['test_code'], + variables: { + SYS_JES_JOB_URI: '', + _program: '/Public/sasjs/jobs/jobs/test_job' + }, + arguments: { + _contextName: 'SAS Job Execution compute context', + _OMITJSONLISTING: true, + _OMITJSONLOG: true, + _OMITSESSIONRESULTS: true, + _OMITTEXTLISTING: true, + _OMITTEXTLOG: true + } + }` + + const resHeaders = ['content-type', 'application/json'] + const resData = { + id: 'id_string', + name: 'name_string', + uri: 'uri_string', + createdBy: 'createdBy_string', + code: 'TEST CODE', + links: [ + { + method: 'method_string', + rel: 'state', + href: 'state_href_string', + uri: 'uri_string', + type: 'type_string' + }, + { + method: 'method_string', + rel: 'state', + href: 'state_href_string', + uri: 'uri_string', + type: 'type_string' + }, + { + method: 'method_string', + rel: 'state', + href: 'state_href_string', + uri: 'uri_string', + type: 'type_string' + }, + { + method: 'method_string', + rel: 'state', + href: 'state_href_string', + uri: 'uri_string', + type: 'type_string' + }, + { + method: 'method_string', + rel: 'state', + href: 'state_href_string', + uri: 'uri_string', + type: 'type_string' + }, + { + method: 'method_string', + rel: 'self', + href: 'self_href_string', + uri: 'uri_string', + type: 'type_string' + } + ], + results: { '_webout.json': '_webout.json_string' }, + logStatistics: { + lineCount: 1, + modifiedTimeStamp: 'modifiedTimeStamp_string' + } + } + beforeAll(() => { + ;(process as any).logger = new Logger(LogLevel.Off) + jest.spyOn((process as any).logger, 'info') + }) + + it('should log parsed response with status 1**', () => { + const spyIsAxiosError = jest + .spyOn(axios, 'isAxiosError') + .mockImplementation(() => true) + + const mockedAxiosError = { + config: { + data: reqData + }, + request: { + _currentRequest: { + _header: reqHeaders + } + } + } as AxiosError + + const requestClient = new RequestClient('') + requestClient['defaultInterceptionCallBack'](mockedAxiosError) + + const noValueMessage = 'Not provided' + const expectedLog = `HTTP Request (first 50 lines): +${reqHeaders}${requestClient['parseInterceptedBody'](reqData)} + +HTTP Response Code: ${requestClient['prettifyString'](noValueMessage)} + +HTTP Response (first 50 lines): +${noValueMessage} +\n${requestClient['parseInterceptedBody'](noValueMessage)} +` + + expect((process as any).logger.info).toHaveBeenCalledWith(expectedLog) + + spyIsAxiosError.mockReset() + }) + + it('should log parsed response with status 2**', () => { + const status = getRandomStatus([ + 200, 201, 202, 203, 204, 205, 206, 207, 208, 226 + ]) + const mockedResponse: AxiosResponse = { data: resData, status, @@ -192,6 +231,138 @@ ${resHeaders[0]}: ${resHeaders[1]}${ expect((process as any).logger.info).toHaveBeenCalledWith(expectedLog) }) + + it('should log parsed response with status 3**', () => { + const status = getRandomStatus([300, 301, 302, 303, 304, 307, 308]) + + const mockedResponse: AxiosResponse = { + data: resData, + status, + statusText: '', + headers: {}, + config: { data: reqData }, + request: { _header: reqHeaders, res: { rawHeaders: resHeaders } } + } + + const requestClient = new RequestClient('') + requestClient['defaultInterceptionCallBack'](mockedResponse) + + const expectedLog = `HTTP Request (first 50 lines): +${reqHeaders}${requestClient['parseInterceptedBody'](reqData)} + +HTTP Response Code: ${requestClient['prettifyString'](status)} + +HTTP Response (first 50 lines): +${resHeaders[0]}: ${resHeaders[1]}${ + requestClient['parseInterceptedBody'](resData) + ? `\n\n${requestClient['parseInterceptedBody'](resData)}` + : '' + } +` + + expect((process as any).logger.info).toHaveBeenCalledWith(expectedLog) + }) + + it('should log parsed response with status 4**', () => { + const spyIsAxiosError = jest + .spyOn(axios, 'isAxiosError') + .mockImplementation(() => true) + + const status = getRandomStatus([ + 400, 401, 402, 403, 404, 407, 408, 409, 410, 411, 412, 413, 414, 415, + 416, 417, 418, 421, 422, 423, 424, 425, 426, 428, 429, 431, 451 + ]) + + const mockedResponse: AxiosResponse = { + data: resData, + status, + statusText: '', + headers: {}, + config: { data: reqData }, + request: { _header: reqHeaders, res: { rawHeaders: resHeaders } } + } + const mockedAxiosError = { + config: { + data: reqData + }, + request: { + _currentRequest: { + _header: reqHeaders + } + }, + response: mockedResponse + } as AxiosError + + const requestClient = new RequestClient('') + requestClient['defaultInterceptionCallBack'](mockedAxiosError) + + const expectedLog = `HTTP Request (first 50 lines): +${reqHeaders}${requestClient['parseInterceptedBody'](reqData)} + +HTTP Response Code: ${requestClient['prettifyString'](status)} + +HTTP Response (first 50 lines): +${resHeaders[0]}: ${resHeaders[1]}${ + requestClient['parseInterceptedBody'](resData) + ? `\n\n${requestClient['parseInterceptedBody'](resData)}` + : '' + } +` + + expect((process as any).logger.info).toHaveBeenCalledWith(expectedLog) + + spyIsAxiosError.mockReset() + }) + + it('should log parsed response with status 5**', () => { + const spyIsAxiosError = jest + .spyOn(axios, 'isAxiosError') + .mockImplementation(() => true) + + const status = getRandomStatus([ + 500, 501, 502, 503, 504, 505, 506, 507, 508, 510, 511 + ]) + + const mockedResponse: AxiosResponse = { + data: resData, + status, + statusText: '', + headers: {}, + config: { data: reqData }, + request: { _header: reqHeaders, res: { rawHeaders: resHeaders } } + } + const mockedAxiosError = { + config: { + data: reqData + }, + request: { + _currentRequest: { + _header: reqHeaders + } + }, + response: mockedResponse + } as AxiosError + + const requestClient = new RequestClient('') + requestClient['defaultInterceptionCallBack'](mockedAxiosError) + + const expectedLog = `HTTP Request (first 50 lines): +${reqHeaders}${requestClient['parseInterceptedBody'](reqData)} + +HTTP Response Code: ${requestClient['prettifyString'](status)} + +HTTP Response (first 50 lines): +${resHeaders[0]}: ${resHeaders[1]}${ + requestClient['parseInterceptedBody'](resData) + ? `\n\n${requestClient['parseInterceptedBody'](resData)}` + : '' + } +` + + expect((process as any).logger.info).toHaveBeenCalledWith(expectedLog) + + spyIsAxiosError.mockReset() + }) }) describe('enableVerboseMode', () => { @@ -217,12 +388,12 @@ ${resHeaders[0]}: ${resHeaders[1]}${ 'use' ) - const successCallback = (response: AxiosResponse) => { + const successCallback = (response: AxiosResponse | AxiosError) => { console.log('success') return response } - const failureCallback = (response: AxiosResponse) => { + const failureCallback = (response: AxiosResponse | AxiosError) => { console.log('failure') return response @@ -522,3 +693,11 @@ const createCertificate = async (): Promise => { ) }) } + +/** + * Returns a random status code. + * @param statuses - an array of available statuses. + * @returns - random item from an array of statuses. + */ +const getRandomStatus = (statuses: number[]) => + statuses[Math.floor(Math.random() * statuses.length)] From 5b2d9e675f2b1aea9f4fd09ecd96323b0ba50a0b Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Thu, 17 Aug 2023 13:29:57 +0300 Subject: [PATCH 2/3] test: set coverage threshold --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- jest.config.js | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9972b60..e58a67f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -14,7 +14,7 @@ What code changes have been made to achieve the intent. No PR (that involves a non-trivial code change) should be merged, unless all items below are confirmed! If an urgent fix is needed - use a tar file. - +- [ ] Unit tests coverage has been increased and a new threshold is set. - [ ] All `sasjs-cli` unit tests are passing (`npm test`). - (CI Runs this) All `sasjs-tests` are passing. If you want to run it manually (instructions available [here](https://github.com/sasjs/adapter/blob/master/sasjs-tests/README.md)). - [ ] [Data Controller](https://datacontroller.io) builds and is functional on both SAS 9 and Viya diff --git a/jest.config.js b/jest.config.js index 6a81100..47bd7db 100644 --- a/jest.config.js +++ b/jest.config.js @@ -41,7 +41,14 @@ module.exports = { // ], // An object that configures minimum threshold enforcement for coverage results - // coverageThreshold: undefined, + coverageThreshold: { + global: { + statements: 63.66, + branches: 44.74, + functions: 53.94, + lines: 64.12 + } + }, // A path to a custom dependency extractor // dependencyExtractor: undefined, From 259b6b3ff2679f04641e768c6f2d8c800ac24556 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Thu, 17 Aug 2023 13:44:08 +0300 Subject: [PATCH 3/3] test(request-client): revert changed port --- src/test/RequestClient.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/RequestClient.spec.ts b/src/test/RequestClient.spec.ts index cbdbf1d..7bcb521 100644 --- a/src/test/RequestClient.spec.ts +++ b/src/test/RequestClient.spec.ts @@ -27,7 +27,7 @@ jest axiosActual.create({ baseURL, httpsAgent }) ) -const PORT = 8015 +const PORT = 8000 const SERVER_URL = `https://localhost:${PORT}/` describe('RequestClient', () => {