From 8170c037194b7fa8217951df609e593e5e38e672 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Tue, 10 Dec 2024 13:24:35 -0500 Subject: [PATCH] Address PR Comments --- src/constants.ts | 3 + src/identityApiClient.ts | 161 +++++----- test/src/tests-identityApiClient.ts | 452 +++++++++++++++++++++++++++- 3 files changed, 537 insertions(+), 79 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index ebeed00e..96390f5e 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -201,4 +201,7 @@ export const MILLIS_IN_ONE_SEC = 1000; export const HTTP_OK = 200 as const; export const HTTP_ACCEPTED = 202 as const; export const HTTP_BAD_REQUEST = 400 as const; +export const HTTP_UNAUTHORIZED = 401 as const; export const HTTP_FORBIDDEN = 403 as const; +export const HTTP_NOT_FOUND = 404 as const; +export const HTTP_SERVER_ERROR = 500 as const; diff --git a/src/identityApiClient.ts b/src/identityApiClient.ts index 0915e810..304fa99d 100644 --- a/src/identityApiClient.ts +++ b/src/identityApiClient.ts @@ -1,4 +1,4 @@ -import Constants, { HTTP_ACCEPTED, HTTP_OK } from './constants'; +import Constants, { HTTP_ACCEPTED, HTTP_BAD_REQUEST, HTTP_OK } from './constants'; import { AsyncUploader, FetchUploader, @@ -114,54 +114,65 @@ export default function IdentityAPIClient( try { const response: Response = await uploader.upload(uploadPayload); - let message: string; let aliasResponseBody: IAliasResponseBody; - - // FetchUploader returns the response as a JSON object that we have to await - if (response.json) { - // HTTP responses of 202, 200, and 403 do not have a response. - // response.json will always exist on a fetch, but can only be - // await-ed when the response is not empty, otherwise it will - // throw an error. - try { - aliasResponseBody = await response.json(); - } catch (e) { - verbose('The request has no response body'); - } - } else { - // https://go.mparticle.com/work/SQDSDKS-6568 - // XHRUploader returns the response as a string that we need to parse - const xhrResponse = (response as unknown) as XMLHttpRequest; - - aliasResponseBody = xhrResponse.responseText - ? JSON.parse(xhrResponse.responseText) - : ''; - } - + let message: string; let errorMessage: string; switch (response.status) { - // Alias response is a 202 with no body - case HTTP_OK: case HTTP_ACCEPTED: - // https://go.mparticle.com/work/SQDSDKS-6670 - message = - 'Successfully sent forwarding stats to mParticle Servers'; - break; - default: { - // 400 has an error message, but 403 doesn't - const errorResponse: IAliasErrorResponse = aliasResponseBody as unknown as IAliasErrorResponse; - if (errorResponse?.message) { - errorMessage = errorResponse.message; + case HTTP_OK: + + // 400 error will has a body and will go through the happy path to report the error + case HTTP_BAD_REQUEST: + + // FetchUploader returns the response as a JSON object that we have to await + if (response.json) { + // HTTP responses of 202, 200, and 403 do not have a response. + // response.json will always exist on a fetch, but can only be + // await-ed when the response is not empty, otherwise it will + // throw an error. + try { + aliasResponseBody = await response.json(); + } catch (e) { + verbose('The request has no response body'); + } + } else { + // https://go.mparticle.com/work/SQDSDKS-6568 + // XHRUploader returns the response as a string that we need to parse + const xhrResponse = (response as unknown) as XMLHttpRequest; + + aliasResponseBody = xhrResponse.responseText + ? JSON.parse(xhrResponse.responseText) + : ''; + + // https://go.mparticle.com/work/SQDSDKS-6670 + message = + 'Successfully sent forwarding stats to mParticle Servers'; } - message = - 'Issue with sending Alias Request to mParticle Servers, received HTTP Code of ' + - response.status; - - if (errorResponse?.code) { - message += ' - ' + errorResponse.code; + + + if (response.status === HTTP_BAD_REQUEST) { + // 400 has an error message, but 403 doesn't + const errorResponse: IAliasErrorResponse = aliasResponseBody as unknown as IAliasErrorResponse; + if (errorResponse?.message) { + errorMessage = errorResponse.message; + } + message = + 'Issue with sending Alias Request to mParticle Servers, received HTTP Code of ' + + response.status; + + if (errorResponse?.code) { + message += ' - ' + errorResponse.code; + } } + + break; + + // 401 and 403 have no bodies and should be rejected outright + default: { + throw new Error('Received HTTP Code of ' + response.status); } + } verbose(message); @@ -230,42 +241,53 @@ export default function IdentityAPIClient( let identityResponse: IIdentityResponse; let message: string; - // FetchUploader returns the response as a JSON object that we have to await - if (response.json) { - // https://go.mparticle.com/work/SQDSDKS-6568 - // FetchUploader returns the response as a JSON object that we have to await - const responseBody: IdentityResultBody = await response.json(); - - identityResponse = this.getIdentityResponseFromFetch( - response, - responseBody - ); - } else { - identityResponse = this.getIdentityResponseFromXHR( - (response as unknown) as XMLHttpRequest - ); - } - - - switch (identityResponse.status) { - case HTTP_OK: + switch (response.status) { case HTTP_ACCEPTED: - message = 'Received Identity Response from server: '; - message += JSON.stringify(identityResponse.responseText); - break; + case HTTP_OK: - default: { - // 400 has an error message, but 403 doesn't - const errorResponse: IdentityApiErrorResponse = identityResponse.responseText as unknown as IdentityApiErrorResponse; - if (errorResponse?.Errors) { + // 400 error will has a body and will go through the happy path to report the error + case HTTP_BAD_REQUEST: + + // FetchUploader returns the response as a JSON object that we have to await + if (response.json) { + // https://go.mparticle.com/work/SQDSDKS-6568 + // FetchUploader returns the response as a JSON object that we have to await + const responseBody: IdentityResultBody = await response.json(); + + identityResponse = this.getIdentityResponseFromFetch( + response, + responseBody + ); + } else { + identityResponse = this.getIdentityResponseFromXHR( + (response as unknown) as XMLHttpRequest + ); + } + + if (identityResponse.status === HTTP_BAD_REQUEST) { + const errorResponse: IdentityApiErrorResponse = identityResponse.responseText as unknown as IdentityApiErrorResponse; message = 'Issue with sending Identity Request to mParticle Servers, received HTTP Code of ' + identityResponse.status; - const errorMessage = errorResponse.Errors.map((error) => error.message).join(', '); - message += ' - ' + errorMessage; + if (errorResponse?.Errors) { + const errorMessage = errorResponse.Errors.map((error) => error.message).join(', '); + message += ' - ' + errorMessage; + } + + } else { + message = 'Received Identity Response from server: '; + message += JSON.stringify(identityResponse.responseText); } + + break; + + // 401 and 403 have no bodies and should be rejected outright + default: { + throw new Error('Received HTTP Code of ' + response.status); } } + mpInstance._Store.identityCallInFlight = false; + verbose(message); parseIdentityResponse( identityResponse, @@ -280,6 +302,7 @@ export default function IdentityAPIClient( mpInstance._Store.identityCallInFlight = false; const errorMessage = (err as Error).message || err.toString(); + error('Error sending identity request to servers' + ' - ' + errorMessage); invokeCallback( callback, diff --git a/test/src/tests-identityApiClient.ts b/test/src/tests-identityApiClient.ts index 4cc7d060..cafe146f 100644 --- a/test/src/tests-identityApiClient.ts +++ b/test/src/tests-identityApiClient.ts @@ -7,16 +7,20 @@ import { IAliasRequest, IIdentityAPIRequestData, } from '../../src/identity.interfaces'; -import { +import Constants, { HTTP_ACCEPTED, HTTP_BAD_REQUEST, HTTP_FORBIDDEN, + HTTP_NOT_FOUND, HTTP_OK, + HTTP_SERVER_ERROR, + HTTP_UNAUTHORIZED, } from '../../src/constants'; import IdentityAPIClient, { IIdentityApiClient } from '../../src/identityApiClient'; import { IIdentityResponse } from '../../src/identity-user-interfaces'; import Utils from './config/utils'; const { fetchMockSuccess } = Utils; +const { HTTPCodes } = Constants; declare global { interface Window { @@ -25,7 +29,6 @@ declare global { } } -let mockServer; const mParticle = window.mParticle; declare global { @@ -352,9 +355,344 @@ describe('Identity Api Client', () => { body: JSON.stringify(identityRequest), }; - expect(fetchMock.calls()[0][1].method).to.deep.equal(expectedFetchPayload.method); - expect(fetchMock.calls()[0][1].body).to.deep.equal(expectedFetchPayload.body); - expect(fetchMock.calls()[0][1].headers).to.deep.equal(expectedFetchPayload.headers); + expect(fetchMock.calls()[0][1].method, 'Payload Method').to.deep.equal(expectedFetchPayload.method); + expect(fetchMock.calls()[0][1].body, 'Payload Body').to.deep.equal(expectedFetchPayload.body); + expect(fetchMock.calls()[0][1].headers, 'Payload Headers').to.deep.equal(expectedFetchPayload.headers); + }); + + it('should include a detailed error message if the fetch returns a 400 (Bad Request)', async () => { + const identityRequestError = { + "Errors": [ + { + "code": "LOOKUP_ERROR", + "message": "knownIdentities is empty." + } + ], + "ErrorCode": "LOOKUP_ERROR", + "StatusCode": 400, + "RequestId": "6c6a234f-e171-4588-90a2-d7bc02530ec3" + }; + + fetchMock.post(urls.identify, { + status: HTTP_BAD_REQUEST, + body: identityRequestError, + }, { + overwriteRoutes: true, + }); + + const callbackSpy = sinon.spy(); + const invokeCallbackSpy = sinon.spy(); + const verboseSpy = sinon.spy(); + const errorSpy = sinon.spy(); + + const mpInstance: MParticleWebSDK = ({ + Logger: { + verbose: (message) => { + debugger; + return verboseSpy(message); + }, + error: (message) => errorSpy(message), + }, + _Helpers: { + createServiceUrl: () => + 'https://identity.mparticle.com/v1/', + + invokeCallback: (callback, httpCode, errorMessage) => + invokeCallbackSpy(callback, httpCode, errorMessage), + }, + _Store: { + devToken: 'test_key', + SDKConfig: { + identityUrl: '', + }, + }, + _Persistence: {}, + } as unknown) as MParticleWebSDK; + + const identityApiClient: IIdentityApiClient = new IdentityAPIClient( + mpInstance + ); + + const parseIdentityResponseSpy = sinon.spy(); + + await identityApiClient.sendIdentityRequest( + identityRequest, + 'identify', + callbackSpy, + originalIdentityApiData, + parseIdentityResponseSpy, + testMPID, + identityRequest.known_identities + ); + + const expectedIdentityErrorRequest = { + status: 400, + responseText: identityRequestError, + cacheMAxAge: 0, + expireTimestamp: 0, + } + + expect(verboseSpy.lastCall, 'verboseSpy called').to.be.ok; + expect(verboseSpy.lastCall.firstArg).to.equal("Issue with sending Identity Request to mParticle Servers, received HTTP Code of 400 - knownIdentities is empty."); + + // A 400 will still call parseIdentityResponse + expect(parseIdentityResponseSpy.calledOnce, 'parseIdentityResponseSpy').to.eq(true); + expect(parseIdentityResponseSpy.args[0][0].status, 'Identity Error Request Status').to.equal(expectedIdentityErrorRequest.status); + expect(parseIdentityResponseSpy.args[0][0].responseText, 'Identity Error Request responseText').to.deep.equal(expectedIdentityErrorRequest.responseText); + expect(parseIdentityResponseSpy.args[0][1]).to.equal(testMPID); + expect(parseIdentityResponseSpy.args[0][2]).to.be.a('function'); + expect(parseIdentityResponseSpy.args[0][3]).to.deep.equal(originalIdentityApiData); + expect(parseIdentityResponseSpy.args[0][4]).to.equal('identify'); + expect(parseIdentityResponseSpy.args[0][5]).to.deep.equal(identityRequest.known_identities); + expect(parseIdentityResponseSpy.args[0][6]).to.equal(false); + }); + + it('should include a detailed error message if the fetch returns a 401 (Unauthorized)', async () => { + fetchMock.post(urls.identify, { + status: HTTP_UNAUTHORIZED, + body: null, + }, { + overwriteRoutes: true, + }); + + const callbackSpy = sinon.spy(); + const invokeCallbackSpy = sinon.spy(); + const verboseSpy = sinon.spy(); + const errorSpy = sinon.spy(); + + const mpInstance: MParticleWebSDK = ({ + Logger: { + verbose: (message) => verboseSpy(message), + error: (message) => errorSpy(message), + }, + _Helpers: { + createServiceUrl: () => + 'https://identity.mparticle.com/v1/', + + invokeCallback: (callback, httpCode, errorMessage) => + invokeCallbackSpy(callback, httpCode, errorMessage), + }, + _Store: { + devToken: 'test_key', + SDKConfig: { + identityUrl: '', + }, + }, + _Persistence: {}, + } as unknown) as MParticleWebSDK; + + const identityApiClient: IIdentityApiClient = new IdentityAPIClient( + mpInstance + ); + + const parseIdentityResponseSpy = sinon.spy(); + + await identityApiClient.sendIdentityRequest( + identityRequest, + 'identify', + callbackSpy, + originalIdentityApiData, + parseIdentityResponseSpy, + testMPID, + identityRequest.known_identities + ); + + expect(errorSpy.lastCall, 'errorSpy called').to.be.ok; + expect(errorSpy.lastCall.firstArg).to.equal("Error sending identity request to servers - Received HTTP Code of 401"); + + expect(invokeCallbackSpy.calledOnce, 'invokeCallbackSpy').to.eq(true); + expect(invokeCallbackSpy.args[0][0]).to.equal(callbackSpy); + expect(invokeCallbackSpy.args[0][1]).to.equal(-1); + expect(invokeCallbackSpy.args[0][2]).to.equal("Received HTTP Code of 401"); + + // A 401 should not call parseIdentityResponse + expect(parseIdentityResponseSpy.calledOnce, 'parseIdentityResponseSpy').to.eq(false); + }); + + it('should include a detailed error message if the fetch returns a 403 (Forbidden)', async () => { + fetchMock.post(urls.identify, { + status: HTTP_FORBIDDEN, + body: null, + }, { + overwriteRoutes: true, + }); + + const callbackSpy = sinon.spy(); + const invokeCallbackSpy = sinon.spy(); + const verboseSpy = sinon.spy(); + const errorSpy = sinon.spy(); + + const mpInstance: MParticleWebSDK = ({ + Logger: { + verbose: (message) => verboseSpy(message), + error: (message) => errorSpy(message), + }, + _Helpers: { + createServiceUrl: () => + 'https://identity.mparticle.com/v1/', + + invokeCallback: (callback, httpCode, errorMessage) => invokeCallbackSpy(callback, httpCode, errorMessage), + }, + _Store: { + devToken: 'test_key', + SDKConfig: { + identityUrl: '', + }, + }, + _Persistence: {}, + } as unknown) as MParticleWebSDK; + + const identityApiClient: IIdentityApiClient = new IdentityAPIClient( + mpInstance + ); + + const parseIdentityResponseSpy = sinon.spy(); + + await identityApiClient.sendIdentityRequest( + identityRequest, + 'identify', + callbackSpy, + originalIdentityApiData, + parseIdentityResponseSpy, + testMPID, + identityRequest.known_identities + ); + + expect(errorSpy.lastCall, 'errorSpy called').to.be.ok; + expect(errorSpy.lastCall.firstArg).to.equal("Error sending identity request to servers - Received HTTP Code of 403"); + + expect(invokeCallbackSpy.calledOnce, 'invokeCallbackSpy').to.eq(true); + expect(invokeCallbackSpy.args[0][0]).to.equal(callbackSpy); + expect(invokeCallbackSpy.args[0][1]).to.equal(-1); + expect(invokeCallbackSpy.args[0][2]).to.equal("Received HTTP Code of 403"); + + // A 403 should not call parseIdentityResponse + expect(parseIdentityResponseSpy.calledOnce, 'parseIdentityResponseSpy').to.eq(false); + + }); + + it('should include a detailed error message if the fetch returns a 404 (Not Found)', async () => { + fetchMock.post(urls.identify, { + status: HTTP_NOT_FOUND, + body: null, + }, { + overwriteRoutes: true, + }); + + const callbackSpy = sinon.spy(); + const invokeCallbackSpy = sinon.spy(); + const verboseSpy = sinon.spy(); + const errorSpy = sinon.spy(); + + const mpInstance: MParticleWebSDK = ({ + Logger: { + verbose: (message) => verboseSpy(message), + error: (message) => errorSpy(message), + }, + _Helpers: { + createServiceUrl: () => + 'https://identity.mparticle.com/v1/', + + invokeCallback: (callback, httpCode, errorMessage) => invokeCallbackSpy(callback, httpCode, errorMessage), + }, + _Store: { + devToken: 'test_key', + SDKConfig: { + identityUrl: '', + }, + }, + _Persistence: {}, + } as unknown) as MParticleWebSDK; + + const identityApiClient: IIdentityApiClient = new IdentityAPIClient( + mpInstance + ); + + const parseIdentityResponseSpy = sinon.spy(); + + await identityApiClient.sendIdentityRequest( + identityRequest, + 'identify', + callbackSpy, + originalIdentityApiData, + parseIdentityResponseSpy, + testMPID, + identityRequest.known_identities + ); + + expect(errorSpy.lastCall, 'errorSpy called').to.be.ok; + expect(errorSpy.lastCall.firstArg).to.equal("Error sending identity request to servers - Received HTTP Code of 404"); + + expect(invokeCallbackSpy.calledOnce, 'invokeCallbackSpy').to.eq(true); + expect(invokeCallbackSpy.args[0][0]).to.equal(callbackSpy); + expect(invokeCallbackSpy.args[0][1]).to.equal(-1); + expect(invokeCallbackSpy.args[0][2]).to.equal("Received HTTP Code of 404"); + + // A 403 should not call parseIdentityResponse + expect(parseIdentityResponseSpy.calledOnce, 'parseIdentityResponseSpy').to.eq(false); + }); + + it('should include a detailed error message if the fetch returns a 500 (Server Error)', async () => { + fetchMock.post(urls.identify, { + status: HTTP_SERVER_ERROR, + body: { + "Errors": [ + { + "code": "INTERNAL_ERROR", + "message": "An unknown error was encountered." + } + ], + "ErrorCode": "INTERNAL_ERROR", + "StatusCode": 500, + "RequestId": null + }, + }, { + overwriteRoutes: true, + }); + + const callbackSpy = sinon.spy(); + const verboseSpy = sinon.spy(); + const errorSpy = sinon.spy(); + + const mpInstance: MParticleWebSDK = ({ + Logger: { + verbose: (message) => verboseSpy(message), + error: (message) => errorSpy(message), + }, + _Helpers: { + createServiceUrl: () => + 'https://identity.mparticle.com/v1/', + + invokeCallback: () => {}, + }, + _Store: { + devToken: 'test_key', + SDKConfig: { + identityUrl: '', + }, + }, + _Persistence: {}, + } as unknown) as MParticleWebSDK; + + const identityApiClient: IIdentityApiClient = new IdentityAPIClient( + mpInstance + ); + + const parseIdentityResponseSpy = sinon.spy(); + + await identityApiClient.sendIdentityRequest( + identityRequest, + 'identify', + callbackSpy, + originalIdentityApiData, + parseIdentityResponseSpy, + testMPID, + identityRequest.known_identities + ); + + expect(errorSpy.calledOnce, 'errorSpy called').to.eq(true); + + expect(errorSpy.args[0][0]).to.equal('Error sending identity request to servers - Received HTTP Code of 500'); }); }); @@ -427,7 +765,14 @@ describe('Identity Api Client', () => { }; const aliasCallback = sinon.spy(); - fetchMock.post(aliasUrl, HTTP_BAD_REQUEST); + fetchMock.post(aliasUrl, { + status: HTTP_BAD_REQUEST, + body: { + message:"The payload was malformed JSON or had missing fields.", + code:"INVALID_DATA"} + }, { + overwriteRoutes: true + }); await identityApiClient.sendAliasRequest( aliasRequest, @@ -437,9 +782,36 @@ describe('Identity Api Client', () => { const callbackArgs = aliasCallback.getCall(0).args; expect(callbackArgs[0]).to.deep.equal({ httpCode: HTTP_BAD_REQUEST, + message: 'The payload was malformed JSON or had missing fields.', }); }); + it('should have an httpCode and an error message passed to the callback on a 401', async () => { + const mpInstance: MParticleWebSDK = mParticle.getInstance(); + const identityApiClient = new IdentityAPIClient(mpInstance); + const aliasRequest: IAliasRequest = { + destinationMpid: '123', + sourceMpid: '456', + startTime: 10001230123, + endTime: 10001231123, + }; + + const aliasCallback = sinon.spy(); + fetchMock.post(aliasUrl, { + status: HTTP_UNAUTHORIZED, + body: null, + }); + + await identityApiClient.sendAliasRequest( + aliasRequest, + aliasCallback + ); + expect(aliasCallback.calledOnce).to.eq(true); + const callbackArgs = aliasCallback.getCall(0).args; + expect(callbackArgs[0].httpCode).to.equal(HTTPCodes.noHttpCoverage); + expect(callbackArgs[0].message).deep.equal('Received HTTP Code of 401'); + }); + it('should have an httpCode and an error message passed to the callback on a 403', async () => { const mpInstance: MParticleWebSDK = mParticle.getInstance(); const identityApiClient = new IdentityAPIClient(mpInstance); @@ -453,7 +825,7 @@ describe('Identity Api Client', () => { const aliasCallback = sinon.spy(); fetchMock.post(aliasUrl, { status: HTTP_FORBIDDEN, - body: JSON.stringify({ message: 'error' }), + body: null, }); await identityApiClient.sendAliasRequest( @@ -462,10 +834,70 @@ describe('Identity Api Client', () => { ); expect(aliasCallback.calledOnce).to.eq(true); const callbackArgs = aliasCallback.getCall(0).args; - expect(callbackArgs[0]).to.deep.equal({ - httpCode: HTTP_FORBIDDEN, - message: 'error', + expect(callbackArgs[0].httpCode).to.equal(HTTPCodes.noHttpCoverage); + expect(callbackArgs[0].message).deep.equal('Received HTTP Code of 403'); + }); + + it('should have an httpCode and an error message passed to the callback on a 404', async () => { + const mpInstance: MParticleWebSDK = mParticle.getInstance(); + const identityApiClient = new IdentityAPIClient(mpInstance); + const aliasRequest: IAliasRequest = { + destinationMpid: '123', + sourceMpid: '456', + startTime: 10001230123, + endTime: 10001231123, + }; + + const aliasCallback = sinon.spy(); + fetchMock.post(aliasUrl, { + status: HTTP_NOT_FOUND, + body: null, + }); + + await identityApiClient.sendAliasRequest( + aliasRequest, + aliasCallback + ); + expect(aliasCallback.calledOnce).to.eq(true); + const callbackArgs = aliasCallback.getCall(0).args; + expect(callbackArgs[0].httpCode).to.equal(HTTPCodes.noHttpCoverage); + expect(callbackArgs[0].message).deep.equal('Received HTTP Code of 404'); + }); + + it('should have an httpCode and an error message passed to the callback on a 500', async () => { + const mpInstance: MParticleWebSDK = mParticle.getInstance(); + const identityApiClient = new IdentityAPIClient(mpInstance); + const aliasRequest: IAliasRequest = { + destinationMpid: '123', + sourceMpid: '456', + startTime: 10001230123, + endTime: 10001231123, + }; + + const aliasCallback = sinon.spy(); + fetchMock.post(aliasUrl, { + status: HTTP_SERVER_ERROR, + body: { + "Errors": [ + { + "code": "INTERNAL_ERROR", + "message": "An unknown error was encountered." + } + ], + "ErrorCode": "INTERNAL_ERROR", + "StatusCode": 500, + "RequestId": null + }, }); + + await identityApiClient.sendAliasRequest( + aliasRequest, + aliasCallback + ); + expect(aliasCallback.calledOnce).to.eq(true); + const callbackArgs = aliasCallback.getCall(0).args; + expect(callbackArgs[0].httpCode).to.equal(HTTPCodes.noHttpCoverage); + expect(callbackArgs[0].message).deep.equal('Received HTTP Code of 500'); }); }); });