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

Add X-Kinvey-Request-Id to errors #84

Merged
merged 3 commits into from Dec 13, 2016

Conversation

Projects
None yet
3 participants
@thomasconner
Contributor

thomasconner commented Dec 9, 2016

Description

This PR adds X-Kinvey-Request-Id to errors. You can view the X-Kinvey-Request-Id related to a specific error by accessing the kinveyRequestId property on an error object.

asyncNetworkCall()
  .catch(function(error) {
    // error.kinveyRequestId
  });

Changes

  • Add X-Kinvey-Request-Id to error objects.
  • Add unit tests to check if X-Kinvey-Request-Id is set properly on an error object.

@thomasconner thomasconner self-assigned this Dec 9, 2016

@thomasconner thomasconner requested a review from tejasranade Dec 9, 2016

@tejasranade

Tests can be improved. Otherwise looks good. Also some questions around docs that'll need a DevCenter PR.

Show outdated Hide outdated test/unit/errors/activeUser.test.js Outdated
@@ -22,6 +23,7 @@ import SyncError from './src/sync';
// Export
export {
ActiveUserError,
BaseError,

This comment has been minimized.

@tejasranade

tejasranade Dec 12, 2016

Member

What is the reason for BaseError to be in the exports?

@tejasranade

tejasranade Dec 12, 2016

Member

What is the reason for BaseError to be in the exports?

This comment has been minimized.

@thomasconner

thomasconner Dec 13, 2016

Contributor

Right now nothing uses it but could be in the future. Just good practice. I can remove it if necessary.

@thomasconner

thomasconner Dec 13, 2016

Contributor

Right now nothing uses it but could be in the future. Just good practice. I can remove it if necessary.

@@ -1,7 +1,7 @@
import BaseError from './base';
export default class ActiveUserError extends BaseError {
constructor(message = 'An active user already exists.', debug, code) {
super('ActiveUserError', message, debug, code);
constructor(message = 'An active user already exists.', debug, code, kinveyRequestId) {

This comment has been minimized.

@tejasranade

tejasranade Dec 12, 2016

Member

For a client-only error like the ActiveUserError, what should I (the developer) expect the value of kinveyRequestId to be? Is this documented?

@tejasranade

tejasranade Dec 12, 2016

Member

For a client-only error like the ActiveUserError, what should I (the developer) expect the value of kinveyRequestId to be? Is this documented?

This comment has been minimized.

@thomasconner

thomasconner Dec 13, 2016

Contributor

They should expect the kinveyRequestId to be undefined. It will need to be documented.

@thomasconner

thomasconner Dec 13, 2016

Contributor

They should expect the kinveyRequestId to be undefined. It will need to be documented.

This comment has been minimized.

@tejasranade

tejasranade Dec 13, 2016

Member

Makes sense. Please open a devcenter PR to go with this.

@tejasranade

tejasranade Dec 13, 2016

Member

Makes sense. Please open a devcenter PR to go with this.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 13, 2016

Current coverage is 65.01% (diff: 53.48%)

No coverage report found for MLIBZ-1507 at cd0836b.

Powered by Codecov. Last update cd0836b...5aa95da

codecov-io commented Dec 13, 2016

Current coverage is 65.01% (diff: 53.48%)

No coverage report found for MLIBZ-1507 at cd0836b.

Powered by Codecov. Last update cd0836b...5aa95da

@tejasranade

LGTM.

@thomasconner thomasconner merged commit 02e367c into MLIBZ-1507 Dec 13, 2016

1 of 2 checks passed

codecov/patch 53.48% of diff hit (target 65.01%)
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@thomasconner thomasconner deleted the MLIBZ-1494 branch Dec 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment