From efde74be2979f58faec965545bcc17d7ec0321f8 Mon Sep 17 00:00:00 2001 From: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Date: Fri, 20 Nov 2020 10:29:19 -0500 Subject: [PATCH 1/2] fix(@aws-amplify/analytics)!: do not attempt to delete unused endpoints BREAKING CHANGE: Analytics will no longer attempt to automatically delete old endpoints on updateEndpoint --- .../AWSPinpointProvider-unit-test.ts | 18 +-- .../src/Providers/AWSPinpointProvider.ts | 104 +----------------- 2 files changed, 4 insertions(+), 118 deletions(-) diff --git a/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts b/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts index 4b60c7ed405..c166ffdf0f6 100644 --- a/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts +++ b/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts @@ -749,33 +749,17 @@ describe('AnalyticsProvider test', () => { // Reject with error the first time we execute updateEndpoint .mockImplementationOnce(async params => { throw mockExceededMaxError; - }) - // Succeed on the second attempt (i.e. after we go through _retryEndpointUpdate) - .mockImplementationOnce(async params => { - return 'data'; }); jest .spyOn(Credentials, 'get') .mockImplementationOnce(() => Promise.resolve(credentials)); - jest - .spyOn(AnalyticsProvider.prototype, '_removeUnusedEndpoints') - .mockImplementationOnce(() => ({ - promise: jest.fn().mockResolvedValue(true), - })); - - const spyonRetryEndpointUpdate = jest.spyOn( - AnalyticsProvider.prototype, - '_retryEndpointUpdate' - ); - const params = { event: { name: '_update_endpoint', immediate: true } }; await analytics.record(params, { resolve, reject }); - expect(spyonUpdateEndpoint).toHaveBeenCalledTimes(2); // 1 failed + 1 successful call - expect(spyonRetryEndpointUpdate).toHaveBeenCalled(); + expect(spyonUpdateEndpoint).toHaveBeenCalledTimes(1); // 1 failed + 1 successful call spyonUpdateEndpoint.mockRestore(); }); diff --git a/packages/analytics/src/Providers/AWSPinpointProvider.ts b/packages/analytics/src/Providers/AWSPinpointProvider.ts index 6515c7004cf..7dbf26c6e80 100644 --- a/packages/analytics/src/Providers/AWSPinpointProvider.ts +++ b/packages/analytics/src/Providers/AWSPinpointProvider.ts @@ -14,7 +14,6 @@ import { ConsoleLogger as Logger, ClientDevice, - Platform, Credentials, Signer, JS, @@ -26,7 +25,6 @@ import { PutEventsCommand, PutEventsCommandInput, UpdateEndpointCommand, - GetUserEndpointsCommand, } from '@aws-sdk/client-pinpoint'; import { EventsBatch } from '@aws-sdk/client-pinpoint/models'; import Cache from '@aws-amplify/cache'; @@ -55,7 +53,6 @@ const logger = new Logger('AWSPinpointProvider'); const RETRYABLE_CODES = [429, 500]; const ACCEPTED_CODES = [202]; const FORBIDDEN_CODE = 403; -const BAD_REQUEST_CODE = 400; const MOBILE_SERVICE_NAME = 'mobiletargeting'; const EXPIRED_TOKEN_CODE = 'ExpiredTokenException'; const UPDATE_ENDPOINT = '_update_endpoint'; @@ -433,11 +430,9 @@ export class AWSPinpointProvider implements AnalyticsProvider { const { err, endpointObject } = failureData; const statusCode = err.$metadata && err.$metadata.httpStatusCode; - logger.debug('updateEndpoint failed', err); + logger.debug('updateEndpoint error', err); switch (statusCode) { - case BAD_REQUEST_CODE: - return this._handleEndpointUpdateBadRequest(failureData); case FORBIDDEN_CODE: return this._handleEndpointUpdateForbidden(failureData); default: @@ -446,41 +441,11 @@ export class AWSPinpointProvider implements AnalyticsProvider { const exponential = true; return this._retryEndpointUpdate(endpointObject, exponential); } + logger.error('updateEndpoint failed', err); endpointObject.handlers.reject(err); } } - private async _handleEndpointUpdateBadRequest( - failureData: EndpointFailureData - ) { - const { err, update_params, endpointObject } = failureData; - const { message } = err; - const { ApplicationId, EndpointRequest } = update_params; - - if ( - !String(message).startsWith('Exceeded maximum endpoint per user count') - ) { - return endpointObject.handlers.reject(err); - } - - try { - await this._removeUnusedEndpoints( - ApplicationId, - EndpointRequest.User.UserId - ); - logger.debug('Removed unused endpoints successfully'); - this._retryEndpointUpdate(endpointObject); - } catch (err) { - logger.warn(`Failed to remove unused endpoints with error: ${err}`); - logger.warn( - `Please ensure you have updated your Pinpoint IAM Policy ` + - `with the Action: "mobiletargeting:GetUserEndpoints" ` + - `in order to get endpoints info of the user` - ); - return endpointObject.handlers.reject(err); - } - } - private _handleEndpointUpdateForbidden(failureData: EndpointFailureData) { const { err, endpointObject } = failureData; @@ -528,69 +493,6 @@ export class AWSPinpointProvider implements AnalyticsProvider { } } - private async _removeUnusedEndpoints(appId, userId) { - try { - // TODO: re-write with Promise (during refactor pt. 2) - const command: GetUserEndpointsCommand = new GetUserEndpointsCommand({ - ApplicationId: appId, - UserId: userId, - }); - const data = await this.pinpointClient.send(command); - const endpoints = data.EndpointsResponse.Item; - logger.debug( - `get endpoints associated with the userId: ${userId} with data`, - endpoints - ); - let endpointToBeDeleted = endpoints[0]; - for (let i = 1; i < endpoints.length; i++) { - const timeStamp1 = Date.parse(endpointToBeDeleted['EffectiveDate']); - const timeStamp2 = Date.parse(endpoints[i]['EffectiveDate']); - // delete the one with invalid effective date - if (isNaN(timeStamp1)) break; - if (isNaN(timeStamp2)) { - endpointToBeDeleted = endpoints[i]; - break; - } - - if (timeStamp2 < timeStamp1) { - endpointToBeDeleted = endpoints[i]; - } - } - // update the endpoint's user id with an empty string - const update_params = { - ApplicationId: appId, - EndpointId: endpointToBeDeleted['Id'], - EndpointRequest: { - User: { - UserId: '', - }, - }, - }; - - try { - const updateEndPointcommand: UpdateEndpointCommand = new UpdateEndpointCommand( - update_params - ); - const updateEndPointData = await this.pinpointClient.send( - updateEndPointcommand - ); - logger.debug( - 'The old endpoint is updated with an empty string for user id' - ); - return updateEndPointData; - } catch (err) { - logger.debug('Failed to update the endpoint', err); - throw err; - } - } catch (err) { - logger.debug( - `Failed to get endpoints associated with the userId: ${userId} with error`, - err - ); - throw err; - } - } - /** * @private * @param config @@ -621,7 +523,7 @@ export class AWSPinpointProvider implements AnalyticsProvider { credentials, customUserAgent: getAmplifyUserAgent(), }); - + // TODO: remove this middleware once a long term fix is implemented by aws-sdk-js team. this.pinpointClient.middlewareStack.addRelativeTo( next => args => { From b5e053aa88cd5b8852bca2fcb6fd445b0096d32a Mon Sep 17 00:00:00 2001 From: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Date: Fri, 20 Nov 2020 12:43:35 -0500 Subject: [PATCH 2/2] commit suggestion Co-authored-by: Manuel Iglesias <6154160+manueliglesias@users.noreply.github.com> --- .../__tests__/Providers/AWSPinpointProvider-unit-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts b/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts index c166ffdf0f6..138db8698fe 100644 --- a/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts +++ b/packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts @@ -759,7 +759,7 @@ describe('AnalyticsProvider test', () => { await analytics.record(params, { resolve, reject }); - expect(spyonUpdateEndpoint).toHaveBeenCalledTimes(1); // 1 failed + 1 successful call + expect(spyonUpdateEndpoint).toHaveBeenCalledTimes(1); spyonUpdateEndpoint.mockRestore(); });