Skip to content

Commit

Permalink
Handle Pinpoint error without "message" (aws-amplify#4507)
Browse files Browse the repository at this point in the history
* Handle undefined `message`

From our remote logs we saw errors coming from this line `TypeError: Cannot read property 'startsWith' of undefined`. We are unable to check more deeply what was the response because we do not not log payloads. I'm making this change in order to make the code more failsafe

* Test was was incorrectly calling "callback" instead of asserting rejection

* Prettier formatting

* Add unit test for BAD_REQUEST_CODE

* Cast message as String vs. defaulting value
  • Loading branch information
ericclemmons authored and elorzafe committed Dec 18, 2019
1 parent 641e991 commit 26755b0
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -730,12 +730,14 @@ describe('AnalyticsProvider test', () => {

test('error case', async () => {
const analytics = new AnalyticsProvider();
const mockError = { message: 'error' };

analytics.configure(options);
const spyon = jest
.spyOn(Pinpoint.prototype, 'updateEndpoint')
.mockImplementationOnce((params, callback) => {
callback({ message: 'error' }, null);
});
.mockImplementationOnce(params => ({
promise: jest.fn().mockRejectedValue(mockError),
}));

jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
return Promise.resolve(credentials);
Expand All @@ -744,7 +746,29 @@ describe('AnalyticsProvider test', () => {
const params = { event: { name: '_update_endpoint', immediate: true } };

await analytics.record(params, { resolve, reject });
expect(reject).toBeCalled();
expect(reject).toBeCalledWith(mockError);
spyon.mockRestore();
});

test('BAD_REQUEST_CODE without message rejects error', async () => {
const analytics = new AnalyticsProvider();
const mockError = { debug: 'error', statusCode: 400 };

analytics.configure(options);
const spyon = jest
.spyOn(Pinpoint.prototype, 'updateEndpoint')
.mockImplementationOnce(params => ({
promise: jest.fn().mockRejectedValue(mockError),
}));

jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
return Promise.resolve(credentials);
});

const params = { event: { name: '_update_endpoint', immediate: true } };

await analytics.record(params, { resolve, reject });
expect(reject).toBeCalledWith(mockError);
spyon.mockRestore();
});
});
Expand Down
16 changes: 6 additions & 10 deletions packages/analytics/src/Providers/AWSPinpointProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,7 @@ export default class AWSPinpointProvider implements AnalyticsProvider {
typeof params.resendLimit === 'number' ? params.resendLimit : resendLimit;
if (params.resendLimit-- > 0) {
logger.debug(
`resending event ${params.eventName} with ${
params.resendLimit
} retry times left`
`resending event ${params.eventName} with ${params.resendLimit} retry times left`
);
this._pinpointPutEvents(params, handlers);
} else {
Expand Down Expand Up @@ -450,7 +448,9 @@ export default class AWSPinpointProvider implements AnalyticsProvider {
const { message } = err;
const { ApplicationId, EndpointRequest } = update_params;

if (!message.startsWith('Exceeded maximum endpoint per user count')) {
if (
!String(message).startsWith('Exceeded maximum endpoint per user count')
) {
return endpointObject.handlers.reject(err);
}

Expand Down Expand Up @@ -494,19 +494,15 @@ export default class AWSPinpointProvider implements AnalyticsProvider {

if (params.resendLimit-- > 0) {
logger.debug(
`resending endpoint update ${params.event.eventId} with ${
params.resendLimit
} retry attempts remaining`
`resending endpoint update ${params.event.eventId} with ${params.resendLimit} retry attempts remaining`
);
// insert at the front of endpointBuffer
this._endpointBuffer.unshift(endpointObject);
return;
}

logger.warn(
`resending endpoint update ${params.event.eventId} failed after ${
params.config.resendLimit
} attempts`
`resending endpoint update ${params.event.eventId} failed after ${params.config.resendLimit} attempts`
);

if (this._endpointGenerating) {
Expand Down

0 comments on commit 26755b0

Please sign in to comment.