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

1993 consolidating duplicate errors #2463

Merged
merged 12 commits into from
May 5, 2023

Conversation

hui-an-yang
Copy link
Collaborator

@hui-an-yang hui-an-yang commented May 3, 2023

closes #1993

Thank you for your contribution to Taquito.

Before submitting this PR, please make sure:

  • Your code builds cleanly without any errors or warnings
  • You have run the linter against the changes
  • You have added unit tests (if relevant/appropriate)
  • You have added integration tests (if relevant/appropriate)
  • All public methods or types have TypeDoc coverage with a complete description, and ideally an @example
  • You have added or updated corresponding documentation
  • If relevant, you have written a first draft summary describing the change for inclusion in Release Notes.

In this PR, please also make sure:

  • You have linked this PR to the issue by putting closes #TICKETNUMBER in the description box (when applicable)
  • You have added a concise description on your changes

Release Note Draft Snippet

If relevant, please write a summary of your change that will be suitable for
inclusion in the Release Notes for the next Taquito release.

Consolidated common error classes InvalidAddressError , InvalidBlockHashError , InvalidDerivationPathError , InvalidHexStringError , InvalidMessageError , and InvalidViewParameterError , into @taquito/core package

@github-actions
Copy link

github-actions bot commented May 3, 2023

New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/.

Published packages:

npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/

@hui-an-yang hui-an-yang marked this pull request as ready for review May 3, 2023 18:07
@hui-an-yang hui-an-yang self-assigned this May 3, 2023
@hui-an-yang hui-an-yang linked an issue May 3, 2023 that may be closed by this pull request
@@ -68,6 +68,7 @@
]
},
"dependencies": {
"@taquito/core": "^16.1.2-beta-RC.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think master is already at 16.1.2, could you merge master into this branch for the latest versions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do

@@ -421,10 +421,10 @@ export class RpcContractProvider
*/
async transferTicket(params: TransferTicketParams) {
if (validateAddress(params.destination) !== ValidationResult.VALID) {
throw new InvalidAddressError(params.destination, 'param destination');
throw new InvalidAddressError(params.destination, 'params destination');
Copy link
Contributor

Choose a reason for hiding this comment

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

I notice the errorDetail param being passed as params destination

How would the error message look like when an errorDetail is passed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for now Address "${address}" is invalid. params destination
I understand it's not ideal, am looking to make it Address "${address}" is invalid. "INVALID_CHECKSUM" found in transferTicket params destination
Will need a bigger undertaking to show why it's invalid, thinking to do that in task3

Copy link
Contributor

Choose a reason for hiding this comment

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

personally I find the params destination bit could be a bit misleading if people don't know which part of the code throws it. I think we can omit it for now. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will this help? Address "${address}" is invalid. in transferTicket params destination?
Was added there cuz the params can have InvalidAddressError with params source or destination
But I don't mind omit it for now to revisit them in task3

Copy link
Contributor

Choose a reason for hiding this comment

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

I think displaying the PKH is clear enough to pinpoint which parameter field was invalid.

And in cases where both source and destination are the same, the errorDetail would be a bit redundant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

they're all removed

@hui-an-yang hui-an-yang requested a review from dsawali May 4, 2023 17:58
@hui-an-yang hui-an-yang merged commit 282ec7d into error-improvement May 5, 2023
@hui-an-yang hui-an-yang deleted the 1993-consolidating-duplicate-errors branch May 5, 2023 20:12
hui-an-yang added a commit that referenced this pull request May 23, 2023
* 1992 create high level error categories (#2452)

* build: created new package @taquito/core

re #1992

* feat: added high-level error categories with unit tests

re #1992

* build: new package-lock.json with @taquito-core for ci

* build: updated a file name and adjusted build order

* test: rename test file

* chore: bump @taquito/core version to 16.1.2

* 1993 consolidating duplicate errors (#2463)

* refactor: consolidated InvalidAddressError into @taquito/core

re #1993

* refactor: consolidated InvalidBlockHashError into @taquito/core

re #1993

* refactor: consolidated InvalidDerivationPathError into @taquito/core

re #1993

* refactor: added InvalidDerivationPathError unit test in @taquito/core

re #1993

* refactor: consolidated InvalidHexStringError in to @taquito/core and some style patches

re #1993

* refactor: consolidated InvalidMessageError into @taquito/core

re #1993

* refactor: consolidated InvalidViewParameterError into @taquito/core

re #1993

* refactor: improve InvalidViewParameterError in @taquito/core

re #1993

* refactor: removed tailling params destination/source in InvalidAddressError

* 1994 @taquito utils error improvement (#2475)

* refactor: consolidate InvalidKeyError inot @taquito/core and improve across packages

BREAKING CHANGE: Class SaplingToolkit function prepareUnshieldedTransaction used to throw
InvalidKeyError now throw a InvalidAddressError instead

re #1994

* refactor: consolidate InvalidPublicKey into @taquito/core and improve across packages

re #1994

* refactor: consolidate InvalidSignatureError into @taquito/core and improve across packages

re #1994

* refactor: minor update of the improved error classes

re #1994

* refactor: consolidate InvalidcontractAddressError into @taquito/core and improve across packages

re #1994

* refactor: consolidate InvalidChainIdError into @taquito/core and improve across packages

re #1994

* refactor: consolidate InvalidKeyHashError into @taquito/core and improve across packages

re #1994

* refactor: minor update for InvalidKeyHashError

re #1994

* refactor: consolidate InvalidOperationHashError into @taquito/core and improve across packages

re #1994

* refactor: consolidate InvalidOperationKindError into @taquito/core and improve across packages

re #1994

* refactor: consolidate DeprecationError into @taquito/core

re #1994

* refactor: consolidate ProhibitedActionError into @taquito/core

re #1994

* refactor: imporve InvalidProtocolHashError and ValueConversionError in @taquito/utils

re #1994

* refactor: added unit tests for error classes in @taquito/core

re #1994

* refactor: consolidate ValidationResult into @taquito/core

re #1994

* refactor: improve InvalidAddressError to show validation result in errorDetail

BREAKING CHANGE: 1. In @taquito/rpc when validateContractAddress used to throw InvalidAddressError
will now throw InvalidContractAddressError. 2. In @taquito/sapling prepareUnshieldedTransaction
function when validateDestinationImplicitAddress used to throw InvalidAddressError now throw
InvalidKeyHashError

re #1994

* refactor: improve InvalidAddressError message format to be concise

re #1994

* refactor: improve InvalidBlockHashError message format to be concise

re #1994

* refactor: improve InvalidDerivationPathError message format to be consistent

re #1994

* refactor: improve InvalidHexStringError message format

re #1994

* refactor: improve InvalidMessageError message format

re #1994

* refactor: improve InvalidViewParameterError message format

re #1994

* refactor: improve InvalidKeyError message format

re #1994

* refactor: improve teh rest of common error classes message format

* refactor: replaced deprecated substr with substring across packages

* refactor: moved validationResult back to @taquito/core

* refactor: addressed pr comments of wording and naming

* 1994 taquito local forging error improvement (#2486)

* refactor: update file name of error.ts to errors.ts for consistency

* refactor: improve InvalidOperationSchemaError

* refactor: improve OversizedEntryPointError

* refactor: improve InvalidBallotValueError

* refactor: improve DecodeBallotValueError

* refactor: imrpove UnexpectedMichelsonValueError

* refactor: improve OperationDecodingError

* refactor: imporve OperationEncodingError

* refactor: imrpove UnsupportedOperationError

* refactor: improve DecodePvmKindError

* refactor: improve InvalidSmartRollupAddressError

* refactor: improve InvalidSmartRollupCommitmentHashError and DecodePvmKindError

BREAKING CHANGE: 1. In @taquito/local-forging smartRollupAddressDecoder used to throw InvalidAddressError now throw invalidContractAddressErro 2. In @taquito/local-forging used to have class InvalidSmartRollupContractAddressError now is InvalidSmartRollupCommitmentHashError 3. In @taquito/local-forging function smartRollupContractAddressEncoder rename to smartRollupCommitmentHashEncoder

* 1994 taquito signer error improvement (#2490)

* refactor: imrpove InvalidMnemonicError

* refactor: improve InvalidBitSize and InvalidCurveError

* refactor: imporve InvalidSeedLengthError

* refactor: replace PrivateKeyError with InvalidKeyError from @taquito/core and improve InvalidKeyErro

BREAKING CHANGE: PrivateKeyError is replaced by common error InvalidKeyError from @taquito/core

* refactor: imrpove ToBeImplemented

* docs: added typedoc of InvalidSeedLengthError and ToBeImplemented

* refactor: imporve InvalidPassphraseError

* chore: updated pack-lock.json

* fix: fix overlooked merge conflicts

* chore: updated package-lock.json and keep bip39 v3.0.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Consolidate duplicated Errors across packages
2 participants