fix(api-rest): refactor ajax method to not relying on side effects

Author: AllanZhengYP

packages/api-rest/__tests__/RestAPI-test.ts

@@ -5,6 +5,8 @@ import { Signer, Credentials, DateUtils } from '@aws-amplify/core';
 
 jest.mock('axios');
 
+const mockAxios = jest.spyOn(axios as any, 'default');
+
 axios.CancelToken = <CancelTokenStatic>{
 	source: () => ({ token: null, cancel: null }),
 };
@@ -28,6 +30,7 @@ const config = {
 
 afterEach(() => {
 	jest.restoreAllMocks();
+	mockAxios.mockClear();
 });
 
 describe('Rest API test', () => {
@@ -166,19 +169,11 @@ describe('Rest API test', () => {
 			api.configure(custom_config);
 			const spyon = jest
 				.spyOn(Credentials, 'get')
-				.mockImplementationOnce(() => {
-					return new Promise((res, rej) => {
-						res('cred');
-					});
-				});
+				.mockResolvedValueOnce('cred');
 
 			const spyonRequest = jest
 				.spyOn(RestClient.prototype as any, '_request')
-				.mockImplementationOnce(() => {
-					return new Promise((res, rej) => {
-						res({});
-					});
-				});
+				.mockResolvedValueOnce({});
 			await api.get('apiName', 'path', {});
 
 			expect(spyonRequest).toBeCalledWith(
@@ -221,25 +216,15 @@ describe('Rest API test', () => {
 				session_token: 'token',
 			};
 
-			const spyon = jest.spyOn(Credentials, 'get').mockImplementation(() => {
-				return new Promise((res, rej) => {
-					res(creds);
-				});
-			});
+			const spyon = jest.spyOn(Credentials, 'get').mockResolvedValue(creds);
 
 			const spyonSigner = jest
 				.spyOn(Signer, 'sign')
 				.mockImplementationOnce(() => {
 					return { headers: {} };
 				});
 
-			const spyAxios = jest
-				.spyOn(axios as any, 'default')
-				.mockImplementationOnce(() => {
-					return new Promise((res, rej) => {
-						res(resp);
-					});
-				});
+			mockAxios.mockResolvedValue(resp);
 
 			const init = {
 				timeout: 2500,
@@ -297,13 +282,7 @@ describe('Rest API test', () => {
 					return { headers: {} };
 				});
 
-			const spyAxios = jest
-				.spyOn(axios as any, 'default')
-				.mockImplementationOnce(() => {
-					return new Promise((res, rej) => {
-						res(resp);
-					});
-				});
+			mockAxios.mockResolvedValue(resp);
 
 			const init = {
 				queryStringParameters: {
@@ -363,13 +342,7 @@ describe('Rest API test', () => {
 					return { headers: {} };
 				});
 
-			const spyAxios = jest
-				.spyOn(axios as any, 'default')
-				.mockImplementationOnce(() => {
-					return new Promise((res, rej) => {
-						res(resp);
-					});
-				});
+			mockAxios.mockResolvedValue(resp);
 
 			const init = {
 				queryStringParameters: {
@@ -429,13 +402,7 @@ describe('Rest API test', () => {
 					return { headers: {} };
 				});
 
-			const spyAxios = jest
-				.spyOn(axios as any, 'default')
-				.mockImplementationOnce(() => {
-					return new Promise((res, rej) => {
-						res(resp);
-					});
-				});
+			mockAxios.mockResolvedValue(resp);
 
 			const init = {
 				queryStringParameters: {
@@ -558,18 +525,39 @@ describe('Rest API test', () => {
 
 			jest.spyOn(Credentials, 'get').mockResolvedValue('creds');
 
-			jest
-				.spyOn(RestClient.prototype as any, '_signed')
-				.mockRejectedValueOnce(normalError);
+			// jest
+			// 	.spyOn(RestClient.prototype as any, '_signed')
+			// 	.mockRejectedValueOnce(normalError);
+
+			jest.spyOn(RestClient.prototype as any, '_sign').mockReturnValue({
+				...init,
+				headers: { ...init.headers, Authorization: 'signed' },
+			});
+			mockAxios.mockImplementationOnce(() => {
+				return new Promise((_, rej) => {
+					rej(normalError);
+				});
+			});
 
 			await expect(api.post('url', 'path', init)).rejects.toThrow(normalError);
 
 			// Clock should not be skewed from normal errors
 			expect(DateUtils.getClockOffset()).toBe(0);
 
-			jest
-				.spyOn(RestClient.prototype as any, '_signed')
-				.mockRejectedValueOnce(clockSkewError);
+			// jest
+			// 	.spyOn(RestClient.prototype as any, '_signed')
+			// 	.mockRejectedValueOnce(clockSkewError);
+
+			// mock clock skew error response and successful response after retry
+			mockAxios
+				.mockImplementationOnce(() => {
+					return new Promise((_, rej) => {
+						rej(clockSkewError);
+					});
+				})
+				.mockResolvedValue({
+					data: [{ name: 'Bob' }],
+				});
 
 			await expect(api.post('url', 'path', init)).resolves.toEqual([
 				{ name: 'Bob' },

packages/api-rest/src/RestClient.ts

@@ -180,39 +180,42 @@ export class RestClient {
 			return this._request(params, isAllResponse);
 		}
 
-		// Signing the request in case there credentials are available
-		return this.Credentials.get().then(
-			credentials => {
-				return this._signed({ ...params }, credentials, isAllResponse, {
-					region,
-					service,
-				}).catch(error => {
-					if (DateUtils.isClockSkewError(error)) {
-						const { headers } = error.response;
-						const dateHeader = headers && (headers.date || headers.Date);
-						const responseDate = new Date(dateHeader);
-						const requestDate = DateUtils.getDateFromHeaderString(
-							params.headers['x-amz-date']
-						);
-
-						// Compare local clock to the server clock
-						if (DateUtils.isClockSkewed(responseDate)) {
-							DateUtils.setClockOffset(
-								responseDate.getTime() - requestDate.getTime()
-							);
-
-							return this.ajax(urlOrApiInfo, method, init);
-						}
-					}
-
-					throw error;
-				});
-			},
-			err => {
-				logger.debug('No credentials available, the request will be unsigned');
-				return this._request(params, isAllResponse);
+		let credentials;
+		try {
+			credentials = await this.Credentials.get();
+		} catch (error) {
+			logger.debug('No credentials available, the request will be unsigned');
+			return this._request(params, isAllResponse);
+		}
+		let signedParams;
+		try {
+			signedParams = this._sign({ ...params }, credentials, {
+				region,
+				service,
+			});
+			const response = await axios(signedParams);
+			return isAllResponse ? response : response.data;
+		} catch (error) {
+			logger.debug(error);
+			if (DateUtils.isClockSkewError(error)) {
+				const { headers } = error.response;
+				const dateHeader = headers && (headers.date || headers.Date);
+				const responseDate = new Date(dateHeader);
+				const requestDate = DateUtils.getDateFromHeaderString(
+					signedParams.headers['x-amz-date']
+				);
+
+				// Compare local clock to the server clock
+				if (DateUtils.isClockSkewed(responseDate)) {
+					DateUtils.setClockOffset(
+						responseDate.getTime() - requestDate.getTime()
+					);
+
+					return this.ajax(urlOrApiInfo, method, init);
+				}
 			}
-		);
+			throw error;
+		}
 	}
 
 	/**
@@ -365,7 +368,7 @@ export class RestClient {
 
 	/** private methods **/
 
-	private _signed(params, credentials, isAllResponse, { service, region }) {
+	private _sign(params, credentials, { service, region }) {
 		const { signerServiceInfo: signerServiceInfoParams, ...otherParams } =
 			params;
 
@@ -400,12 +403,7 @@ export class RestClient {
 
 		delete signed_params.headers['host'];
 
-		return axios(signed_params)
-			.then(response => (isAllResponse ? response : response.data))
-			.catch(error => {
-				logger.debug(error);
-				throw error;
-			});
+		return signed_params;
 	}
 
 	private _request(params, isAllResponse = false) {