From 42aec96410c921faa555c66f3249b01f2124a941 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Mon, 20 Dec 2021 10:56:52 +0300 Subject: [PATCH 1/6] feat(pollJobState): improved loggging --- src/api/viya/pollJobState.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/api/viya/pollJobState.ts b/src/api/viya/pollJobState.ts index 6829611..0ac2597 100644 --- a/src/api/viya/pollJobState.ts +++ b/src/api/viya/pollJobState.ts @@ -206,10 +206,11 @@ const doPoll = async ( pollCount++ + const jobHref = postedJob.links.find((l: Link) => l.rel === 'self')!.href + if (pollOptions?.streamLog) { - const jobUrl = postedJob.links.find((l: Link) => l.rel === 'self') const { result: job } = await requestClient.get( - jobUrl!.href, + jobHref, authConfig?.access_token ) @@ -231,7 +232,7 @@ const doPoll = async ( } if (debug && printedState !== state) { - logger.info('Polling job status...') + logger.info(`Polling: ${requestClient.getBaseUrl() + jobHref}/state`) logger.info(`Current job state: ${state}`) printedState = state From 098e7f85908d91dc0cf2db276b6c0fedc0dd5bb3 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Mon, 20 Dec 2021 10:57:53 +0300 Subject: [PATCH 2/6] fix(errors): fixed error handling function --- src/request/RequestClient.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/request/RequestClient.ts b/src/request/RequestClient.ts index 4084525..6594df5 100644 --- a/src/request/RequestClient.ts +++ b/src/request/RequestClient.ts @@ -195,9 +195,7 @@ export class RequestClient implements HttpClient { } ), debug - ).catch((err) => { - throw prefixMessage(err, 'Error while handling error. ') - }) + ) }) } @@ -488,7 +486,7 @@ export class RequestClient implements HttpClient { else return } - throw e + throw prefixMessage(e, 'Error while handling error. ') } protected parseResponse(response: AxiosResponse) { From 2ebd6e11ba416d2444ba7b3a1f2e8ee1c64a7473 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Tue, 21 Dec 2021 11:40:27 +0300 Subject: [PATCH 3/6] test(RequestClient): fix error handling --- src/api/viya/spec/pollJobState.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api/viya/spec/pollJobState.spec.ts b/src/api/viya/spec/pollJobState.spec.ts index 74f39e1..664fb16 100644 --- a/src/api/viya/spec/pollJobState.spec.ts +++ b/src/api/viya/spec/pollJobState.spec.ts @@ -195,7 +195,7 @@ describe('pollJobState', () => { expect((process as any).logger.info).toHaveBeenCalledTimes(4) expect((process as any).logger.info).toHaveBeenNthCalledWith( 1, - 'Polling job status...' + 'Polling: /job/state' ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 2, @@ -203,7 +203,7 @@ describe('pollJobState', () => { ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 3, - 'Polling job status...' + 'Polling: /job/state' ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 4, From 4197ad5aa88575a360f0323495effb4e545bc2a8 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Tue, 21 Dec 2021 11:40:59 +0300 Subject: [PATCH 4/6] test(RequestClient): fix error handling --- src/request/RequestClient.ts | 11 ++++++++--- src/test/RequestClient.spec.ts | 16 ++++++++++++---- src/test/SAS_server_app.ts | 1 + 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/request/RequestClient.ts b/src/request/RequestClient.ts index 6594df5..3e18632 100644 --- a/src/request/RequestClient.ts +++ b/src/request/RequestClient.ts @@ -215,6 +215,7 @@ export class RequestClient implements HttpClient { .post(url, data, { headers, withCredentials: true }) .then((response) => { throwIfError(response) + return this.parseResponse(response) }) .catch(async (e) => { @@ -464,6 +465,8 @@ export class RequestClient implements HttpClient { if (e instanceof LoginRequiredError) { this.clearCsrfTokens() + + throw e } if (response?.status === 403 || response?.status === 449) { @@ -486,7 +489,8 @@ export class RequestClient implements HttpClient { else return } - throw prefixMessage(e, 'Error while handling error. ') + if (e.message) throw e + else throw prefixMessage(e, 'Error while handling error. ') } protected parseResponse(response: AxiosResponse) { @@ -538,8 +542,9 @@ export class RequestClient implements HttpClient { this.httpClient = createAxiosInstance(baseUrl, httpsAgent) - this.httpClient.defaults.validateStatus = (status) => - status >= 200 && status < 401 + this.httpClient.defaults.validateStatus = (status) => { + return status >= 200 && status <= 401 + } } } diff --git a/src/test/RequestClient.spec.ts b/src/test/RequestClient.spec.ts index 961043c..6a64e83 100644 --- a/src/test/RequestClient.spec.ts +++ b/src/test/RequestClient.spec.ts @@ -5,6 +5,8 @@ import { app, mockedAuthResponse } from './SAS_server_app' import { ServerType } from '@sasjs/utils' import SASjs from '../SASjs' import * as axiosModules from '../utils/createAxiosInstance' +import { LoginRequiredError } from '../types/errors' +import { prefixMessage } from '@sasjs/utils/error' const axiosActual = jest.requireActual('axios') @@ -55,8 +57,11 @@ describe('RequestClient', () => { it('should response the POST method with Unauthorized', async () => { await expect( adapter.getAccessToken('clientId', 'clientSecret', 'incorrect') - ).rejects.toThrow( - 'Error while getting access token. Request failed with status code 401' + ).rejects.toEqual( + prefixMessage( + new LoginRequiredError(), + 'Error while getting access token. ' + ) ) }) }) @@ -132,8 +137,11 @@ describe('RequestClient - Self Signed Server', () => { it('should response the POST method with Unauthorized', async () => { await expect( adapter.getAccessToken('clientId', 'clientSecret', 'incorrect') - ).rejects.toThrow( - 'Error while getting access token. Request failed with status code 401' + ).rejects.toEqual( + prefixMessage( + new LoginRequiredError(), + 'Error while getting access token. ' + ) ) }) }) diff --git a/src/test/SAS_server_app.ts b/src/test/SAS_server_app.ts index e4cff7a..aa8aa2c 100644 --- a/src/test/SAS_server_app.ts +++ b/src/test/SAS_server_app.ts @@ -18,6 +18,7 @@ app.get('/', function (req: any, res: any) { app.post('/SASLogon/oauth/token', function (req: any, res: any) { let valid = true + // capture the encoded form data req.on('data', (data: any) => { const resData = data.toString() From f25d9ec09d2eab02ce8347b6b1e7d0719b8d867b Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Tue, 21 Dec 2021 16:41:08 +0300 Subject: [PATCH 5/6] test(RequestClient): cover handleError method --- src/test/RequestClient.spec.ts | 109 ++++++++++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 1 deletion(-) diff --git a/src/test/RequestClient.spec.ts b/src/test/RequestClient.spec.ts index 6a64e83..9f8ec7d 100644 --- a/src/test/RequestClient.spec.ts +++ b/src/test/RequestClient.spec.ts @@ -5,8 +5,14 @@ import { app, mockedAuthResponse } from './SAS_server_app' import { ServerType } from '@sasjs/utils' import SASjs from '../SASjs' import * as axiosModules from '../utils/createAxiosInstance' -import { LoginRequiredError } from '../types/errors' +import { + LoginRequiredError, + AuthorizeError, + NotFoundError, + InternalServerError +} from '../types/errors' import { prefixMessage } from '@sasjs/utils/error' +import { RequestClient } from '../request/RequestClient' const axiosActual = jest.requireActual('axios') @@ -64,6 +70,107 @@ describe('RequestClient', () => { ) ) }) + + describe('handleError', () => { + const requestClient = new RequestClient('https://localhost:8009') + const randomError = 'some error' + + it('should throw an error if could not get confirmUrl', async () => { + const authError = new AuthorizeError('message', 'confirm_url') + + jest + .spyOn(requestClient['httpClient'], 'get') + .mockImplementation(() => Promise.reject(randomError)) + + await expect( + requestClient['handleError'](authError, () => {}) + ).rejects.toEqual(`Error while getting error confirmUrl. ${randomError}`) + }) + + it('should throw an error if authorize form is required', async () => { + const authError = new AuthorizeError('message', 'confirm_url') + + jest + .spyOn(requestClient['httpClient'], 'get') + .mockImplementation(() => + Promise.resolve({ data: '
' }) + ) + + jest + .spyOn(requestClient, 'authorize') + .mockImplementation(() => Promise.reject(randomError)) + + await expect( + requestClient['handleError'](authError, () => {}) + ).rejects.toEqual(`Error while authorizing request. ${randomError}`) + }) + + it('should throw an error from callback function', async () => { + const authError = new AuthorizeError('message', 'confirm_url') + + jest + .spyOn(requestClient['httpClient'], 'get') + .mockImplementation(() => Promise.resolve({ data: '' })) + + await expect( + requestClient['handleError'](authError, () => + Promise.reject(randomError) + ) + ).rejects.toEqual( + `Error while executing callback in handleError. ${randomError}` + ) + }) + + it('should handle error with 403 response status', async () => { + const error = { + response: { + status: 403, + headers: { 'x-csrf-header': 'x-csrf-header' } + } + } + + await expect( + requestClient['handleError'](error, () => Promise.reject(randomError)) + ).rejects.toEqual( + `Error while executing callback in handleError. ${randomError}` + ) + + error.response.headers = {} as unknown as { 'x-csrf-header': string } + requestClient['csrfToken'].headerName = '' + + await expect( + requestClient['handleError'](error, () => Promise.reject(randomError)) + ).rejects.toEqual(error) + }) + + it('should handle error with 404 response status', async () => { + const error = { + response: { + status: 404, + config: { url: 'test url' } + } + } + + await expect( + requestClient['handleError'](error, () => {}) + ).rejects.toEqual(new NotFoundError(error.response.config.url)) + }) + + it('should handle error with 502 response status', async () => { + const error = { + response: { + status: 502 + } + } + + await expect( + requestClient['handleError'](error, () => {}, true) + ).rejects.toEqual(new InternalServerError()) + await expect( + requestClient['handleError'](error, () => {}, false) + ).resolves.toEqual(undefined) + }) + }) }) describe('RequestClient - Self Signed Server', () => { From 4a963ffbf5f27652519177486daf118b17b63ad6 Mon Sep 17 00:00:00 2001 From: Yury Shkoda Date: Wed, 22 Dec 2021 18:46:06 +0300 Subject: [PATCH 6/6] feat(polling-state): improve logging --- src/SessionManager.ts | 2 +- src/api/viya/spec/pollJobState.spec.ts | 7 +++++-- src/test/SessionManager.spec.ts | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/SessionManager.ts b/src/SessionManager.ts index a0c68d7..1f3ff5f 100644 --- a/src/SessionManager.ts +++ b/src/SessionManager.ts @@ -168,7 +168,7 @@ export class SessionManager { ) { if (stateLink) { if (this.debug && !this.printedSessionState.printed) { - logger.info('Polling session status...') + logger.info(`Polling: ${this.serverUrl + stateLink.href}`) this.printedSessionState.printed = true } diff --git a/src/api/viya/spec/pollJobState.spec.ts b/src/api/viya/spec/pollJobState.spec.ts index 664fb16..4cb45b4 100644 --- a/src/api/viya/spec/pollJobState.spec.ts +++ b/src/api/viya/spec/pollJobState.spec.ts @@ -9,7 +9,10 @@ import * as isNodeModule from '../../../utils/isNode' import { PollOptions } from '../../../types' import { WriteStream } from 'fs' +const baseUrl = 'http://localhost' const requestClient = new (>RequestClient)() +requestClient['httpClient'].defaults.baseURL = baseUrl + const defaultPollOptions: PollOptions = { maxPollCount: 100, pollInterval: 500, @@ -195,7 +198,7 @@ describe('pollJobState', () => { expect((process as any).logger.info).toHaveBeenCalledTimes(4) expect((process as any).logger.info).toHaveBeenNthCalledWith( 1, - 'Polling: /job/state' + `Polling: ${baseUrl}/job/state` ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 2, @@ -203,7 +206,7 @@ describe('pollJobState', () => { ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 3, - 'Polling: /job/state' + `Polling: ${baseUrl}/job/state` ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 4, diff --git a/src/test/SessionManager.spec.ts b/src/test/SessionManager.spec.ts index d37305b..f40205a 100644 --- a/src/test/SessionManager.spec.ts +++ b/src/test/SessionManager.spec.ts @@ -89,7 +89,7 @@ describe('SessionManager', () => { expect((process as any).logger.info).toHaveBeenCalledTimes(3) expect((process as any).logger.info).toHaveBeenNthCalledWith( 1, - 'Polling session status...' + `Polling: ${process.env.SERVER_URL}` ) expect((process as any).logger.info).toHaveBeenNthCalledWith( 2,