Fix hardhat-ethers-chai-matchers#7993
Conversation
🦋 Changeset detectedLatest commit: 692a28f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR refactors error handling in hardhat-ethers-chai-matchers by replacing HardhatError instances with more descriptive assertion failures and improving error context preservation.
Changes:
- Replaced
assertHardhatInvariantandHardhatErrorusages withchaiAssert.fail()and descriptive error messages throughout the matchers - Ensured
error.causeis preserved in catch blocks for better error debugging - Added blank line before error message output in
hardhat-mochafor better formatting
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| v-next/hardhat-mocha/src/unhandled-rejection-mocha-hook.ts | Added blank line before error message output |
| v-next/hardhat-ethers-chai-matchers/test/matchers/reverted/revertedWithPanic.ts | Updated tests to use assertThrows instead of assertThrowsHardhatError |
| v-next/hardhat-ethers-chai-matchers/test/matchers/reverted/revertedWithCustomError.ts | Updated tests to use assertThrows instead of assertThrowsHardhatError |
| v-next/hardhat-ethers-chai-matchers/test/matchers/reverted/revertedWith.ts | Updated tests to use assertThrows instead of assertThrowsHardhatError |
| v-next/hardhat-ethers-chai-matchers/test/matchers/reverted/revert.ts | Updated tests to use assertThrows and assertRejects instead of hardhat-specific error assertions |
| v-next/hardhat-ethers-chai-matchers/test/matchers/reverted/legacyReverted.ts | Updated tests to use assertThrows and assertRejects instead of hardhat-specific error assertions |
| v-next/hardhat-ethers-chai-matchers/test/matchers/events.ts | Updated tests to use assertRejects instead of assertRejectsWithHardhatError |
| v-next/hardhat-ethers-chai-matchers/test/matchers/changeTokenBalance.ts | Updated tests and error message expectations to use new assertion format |
| v-next/hardhat-ethers-chai-matchers/test/matchers/changeEtherBalances.ts | Updated tests to use assertThrows instead of assertThrowsHardhatError |
| v-next/hardhat-ethers-chai-matchers/test/matchers/changeEtherBalance.ts | Updated tests and error message expectations to use new assertion format |
| v-next/hardhat-ethers-chai-matchers/src/internal/utils/prevent-chaining.ts | Replaced HardhatError with chaiAssert.fail() for matcher chaining errors |
| v-next/hardhat-ethers-chai-matchers/src/internal/utils/build-assert.ts | Replaced HardhatError with chaiAssert.fail() for assertion errors |
| v-next/hardhat-ethers-chai-matchers/src/internal/utils/balance.ts | Added error cause preservation in balance retrieval error handling |
| v-next/hardhat-ethers-chai-matchers/src/internal/utils/asserts.ts | Replaced assertHardhatInvariant with chaiAssert methods and preserved error causes |
| v-next/hardhat-ethers-chai-matchers/src/internal/utils/account.ts | Replaced HardhatError with chaiAssert.fail() for address validation |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/withArgs.ts | Replaced HardhatError with chaiAssert.fail() for validation errors |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/reverted/utils.ts | Added error cause preservation in decoding error handling |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/reverted/revertedWithPanic.ts | Added error cause preservation in panic code validation |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/reverted/revertedWithCustomError.ts | Replaced HardhatError with chaiAssert.fail() for validation errors |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/reverted/revertedWith.ts | Replaced HardhatError with chaiAssert.fail() for validation errors |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/reverted/revert.ts | Replaced HardhatError with chaiAssert.fail() and improved error messages |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/reverted/legacyReverted.ts | Replaced HardhatError with chaiAssert.fail() for deprecation error |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/emit.ts | Replaced HardhatError with chaiAssert.fail() and preserved error cause |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/changeTokenBalance.ts | Replaced assertHardhatInvariant with chaiAssert methods and improved error messages |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/changeEtherBalances.ts | Replaced HardhatError with chaiAssert.fail() and improved error messages |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/changeEtherBalance.ts | Replaced assertHardhatInvariant with chaiAssert methods and refactored balance retrieval |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/big-number.ts | Replaced HardhatError with chaiAssert.fail() for unknown comparison operation |
| v-next/hardhat-ethers-chai-matchers/src/internal/matchers/addressable.ts | Replaced assertHardhatInvariant with chaiAssert.ok() and improved error message |
| v-next/hardhat-ethers-chai-matchers/package.json | Removed @nomicfoundation/hardhat-errors dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hardhatTotal size of the bundle: List of dependencies (sorted by size) |
|
I'll fix the chai v5 workflow tomorrow. |
| assertIsNotNull( | ||
| receipt, | ||
| "Transaction's receipt cannot be fetched from the network", | ||
| ); |
There was a problem hiding this comment.
This makes sense if we assume chai matchers can never be used against a non-automining chain, are we happy with that assumption?
There was a problem hiding this comment.
I didn't change that assumption here. It was always present. I guess we can create an issue to revisit it if needed. @ChristopherDedominici
Note that these matchers may assume a few extra things that are only present in dev networks.
There was a problem hiding this comment.
I created this PR (still in draft), to address the issue: #7997
There was a problem hiding this comment.
@ChristopherDedominici how does this work in Hardhat 2, did chai matchers always assume you were running against an automine network?
There was a problem hiding this comment.
Yes, in HH2 it works like that. Much of this code was inherited from Waffle, which worked only with Ganache.
There was a problem hiding this comment.
Let's not block this PR on the other one though
…rted/utils.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Fixed the windows CI |
kanej
left a comment
There was a problem hiding this comment.
Leaving the question of supporting non-automine situations aside, this looks good.
Can I check that the trick to improving the missing await error message is that chai has some inbuilt support for this, and that by changing across to chai's assertions we enable that?
|
This PR is a result of investigating #7726 and replaces it
This PR introduces to three fixes to
hardhat-ethers-chai-matchers:assertHardhatInvariantwithassert.failwith descriptive messages. TheassertHardhatInvariantfunction should only be used for things that should always be true. It should not be used to validate inputs or return values.error.causesHardhatErrors from within assertions, as they play nicely with mocha.It also introduces a small formatting improvement to a
hardhat-mochaerror message.