Skip to content
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.

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

fix(analytics): correctly extract statusCode from failed request #5618

Merged
merged 6 commits into from Apr 29, 2020

Conversation

iartemiev
Copy link
Contributor

Issue #, if available:
Fixes #5423

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@sammartinez sammartinez left a comment

Choose a reason for hiding this comment

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

LGTM 🌮, the implementation is good, I believe we need to adjust a couple of the unit tests. So I will keep it approved but should look into why the unit tests are failing

@codecov
Copy link

codecov bot commented Apr 29, 2020

Codecov Report

Merging #5618 into master will decrease coverage by 0.00%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5618      +/-   ##
==========================================
- Coverage   72.92%   72.91%   -0.01%     
==========================================
  Files         197      197              
  Lines       11532    11532              
  Branches     2252     2176      -76     
==========================================
- Hits         8410     8409       -1     
- Misses       2951     2972      +21     
+ Partials      171      151      -20     
Impacted Files Coverage Δ
packages/analytics/src/Providers/EventBuffer.ts 30.55% <0.00%> (ø)
...ges/analytics/src/Providers/AWSPinpointProvider.ts 75.53% <100.00%> (-0.31%) ⬇️
packages/auth/src/OAuth/OAuth.ts 48.12% <0.00%> (ø)
packages/core/src/Credentials.ts 31.48% <0.00%> (ø)
packages/analytics/src/Analytics.ts 66.86% <0.00%> (ø)
packages/datastore/src/sync/outbox.ts 25.00% <0.00%> (ø)
packages/datastore/src/storage/storage.ts 67.59% <0.00%> (ø)
packages/core/src/OAuthHelper/GoogleOAuth.ts 32.65% <0.00%> (ø)
packages/core/src/Util/Reachability.native.ts 37.50% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5db38d...d278d0e. Read the comment docs.

Copy link
Contributor

@sammartinez sammartinez left a comment

Choose a reason for hiding this comment

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

Reapproving, LGTM 🌮

@iartemiev iartemiev merged commit e11e938 into aws-amplify:master Apr 29, 2020
@iartemiev iartemiev deleted the issue5423 branch April 29, 2020 20:16
logger.debug('updateEndpoint failed', err);
const statusCode = err.$metadata && err.$metadata.httpStatusCode;

logger.error('updateEndpoint failed', err);
Copy link

Choose a reason for hiding this comment

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

The endpoints seem to update successfully now, but this message is still in the log and is now throwing an error despite the success:

error message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, that's a regression introduced in this PR. Will restore it to logger.debug

Choose a reason for hiding this comment

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

There is a similar problem with the 'Missing address' warning now turned into an error, but I noticed that your PR reverts that as well.

IMG_9535F2591BC3-1

@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pinpoint: Exceeded maximum endpoint per user count: 10
3 participants