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

MLIBZ-1899: Fix Error Inheritance #136

Merged
merged 3 commits into from
Jul 7, 2017
Merged

Conversation

thomasconner
Copy link
Contributor

Description

This PR fixes error inheritances and removes the use of es6-error.

Changes

@thomasconner thomasconner self-assigned this Jul 7, 2017
@tejasranade
Copy link
Contributor

tejasranade commented Jul 7, 2017

@thomasconner this PR needs some clarification.

  • What was the specific problem that the ticket reported? Was it a problem with one type of error or every type of error? Why weren't the unit tests catching it?
  • Is there any impact on the API? e.g. reading properties of error objects, Typescript definitions etc.
  • I read the MDN recommendation. so I see where this change comes from. However, it seems like there's a lot of duplicate code in each error constructor (everything other than the message). Can this be parameterized?

@thomasconner
Copy link
Contributor Author

@tejasranade

  1. The problem was that the user would do console.log(error) which would not print out the message property but instead the debug, code, and kinveyRequestId properties. This has been reported several times by customers. This is fairly difficult to catch in unit tests since when you access error.message directly it is available and correct. If a customer did console.log(error.message) they would see the message but without reading the devcenter troubleshooting guide most would miss this. This problem occurred with all the errors.
  2. There is no impact on the API.
  3. I was trying to prevent the duplicate code with the solution we had before but I think due to the way things were being inherited (babel transpilation to es5) caused things to get messed up. Errors are very touchy for some reason when it comes to JavaScript and therefore is why the duplicate code is necessary.

@thomasconner thomasconner merged commit 749a559 into master Jul 7, 2017
@thomasconner thomasconner deleted the MlIBZ-1899_error_messages branch July 7, 2017 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants