Skip to content
This repository has been archived by the owner on Jun 7, 2019. It is now read-only.

Extend APIError to have errors from core #919

Merged
merged 5 commits into from
Dec 3, 2018
Merged

Conversation

shuse2
Copy link
Contributor

@shuse2 shuse2 commented Nov 28, 2018

What was the problem?

APIError did not have enough information to show proper error on the user side.

How did I fix it?

Add errors from APIResponse if exists in the error.

How to test it?

cd packages/lisk-client
npm start
client = lisk.APIClient.createTestnetAPIClient()
client.votes.get({ "random": 1 }).catch((err) => console.log(err.message))
client.votes.get({ "random": 1 }).catch((err) => console.log(err.errno))
client.votes.get({ "random": 1 }).catch((err) => console.log(err.errors))

Review checklist

@shuse2 shuse2 self-assigned this Nov 28, 2018
@shuse2 shuse2 changed the base branch from release/2.0.0 to 912-fix_external_lib_def November 28, 2018 14:50
@shuse2 shuse2 changed the base branch from 912-fix_external_lib_def to release/2.0.0 November 30, 2018 11:07
mitsuaki-u
mitsuaki-u previously approved these changes Dec 3, 2018
});
});

it('should reject with "An unknown error has occured." message if there is no data is supplied', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this test case any useful? Because in the following test, the request is rejected with a defined data property, and the expectation for err.message is the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's useful, because data exists but no message, and no data is treated in the different way in the code

@shuse2 shuse2 merged commit 2db7a39 into release/2.0.0 Dec 3, 2018
@shuse2 shuse2 deleted the 916-extend_api_error branch December 3, 2018 11:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants