Skip to content

Handle Pinpoint error without "message" #4507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Dec 6, 2019
Merged

Handle Pinpoint error without "message" #4507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
aa8e3de
Handle undefined `message`
mariotsi Nov 29, 2019
8c89a51
Merge branch 'master' into patch-1
ericclemmons Dec 3, 2019
8b0adbe
Merge branch 'master' into patch-1
ericclemmons Dec 3, 2019
15f2f14
Test was was incorrectly calling "callback" instead of asserting reje…
ericclemmons Dec 3, 2019
19b9af1
Prettier formatting
ericclemmons Dec 3, 2019
870e6f3
Add unit test for BAD_REQUEST_CODE
ericclemmons Dec 3, 2019
a1b13da
Cast message as String vs. defaulting value
ericclemmons Dec 3, 2019
bfd9219
Merge branch 'master' into mariotsi-patch-1
iartemiev Dec 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions packages/analytics/__tests__/Providers/AWSPinpointProvider-unit-test.ts
View file
Open in desktop
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was rejecting with callback is not a function and still passing, since it wasn't mocking this behavior correctly:
https://github.com/aws-amplify/amplify-js/blob/master/packages/analytics/src/Providers/AWSPinpointProvider.ts#L405-L407

});
.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 };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to the fix in a1b13da, this fails.


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
View file
Open in desktop
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