Web3.js: Survey and add tests for all error messages

[cgewecke]

As part of 1.x stabilization, Web3 needs a good behavioral description of when and how its error messages are thrown. This is a pre-condition for any more extensive refactoring because it's common for users to grep the errors and base further logic on them. In many cases the message strings themselves have become a de-facto interface that needs to be supported.

Below is a tracking table - line numbers are based on the repo state at 513116fb05428327df35c8c5c9528a678d6f498c. As things change those numbers will likely float a bit.

| Cov | Package | File | Line | PR |
| ---------- | ------- | ------- | ------- | ------ |
| ❌ | bzz | index.js | 69 | |
| ❌ | core| index.js | 34 | |
| ✅ | helpers | formatters.js | 141 | |
| ✅ | helpers | formatters.js | 150 | |
| ❌ | helpers | formatters.js | 204 | |
| ❌ | helpers | formatters.js | 263 | |
| ✅ | helpers | formatters.js | 471 | |
| ✅ | method | index.js | 36 | |
| ✅ | method | index.js | 128 | |
| ❌ | method | index.js | 275 | |
| ✅ | method | index.js | 287 | |
| ✅ | method | index.js | 345 | |
| ✅ | method | index.js | 386 | |
| ✅ | method | index.js | 432 | |
| ✅ | method | index.js | 447 | |
| ❌ | method | index.js | 457 | |
| ❌ | method | index.js | 484 | |
| ✅ | method | index.js | 494 | |
| ❌ | method | index.js | 507 | |
| ✅ | method | index.js | 581 | |
| ✅ | requestmngr | batch.js | 57 | |
| ❌ | requestmngr | batch.js | 61 | |
| ❌ | requestmngr | givenprovider.js | 56 | |
| ❌ | requestmngr | givenprovider.js | 68 | |
| ✅ | requestmngr | index.js | 85 | |
| ❌ | requestmngr | index.js | 119 | |
| ✅ | requestmngr | index.js | 127 | |
| ❌ | requestmngr | index.js | 158 | |
| ✅ | requestmngr | index.js | 170 | |
| ✅ | requestmngr | index.js | 174 | |
| ❌ | requestmngr | index.js | 196 | |
| ❌ | requestmngr | jsonrpc.js | 42 | |
| ✅ | core-subscriptions | subscription.js | 81 | #3190 |
| ❌ | core-subscriptions | subscription.js | 158 | |
| ✅ | core-subscriptions | subscription.js | 198 | #3190 |
| ✅ | core-subscriptions | subscription.js | 206 | #3190 |
| ✅ | eth-abi | index.js | 239 | |
| ❌ | eth-accounts | index.js | 82 | |
| ✅ | eth-accounts | index.js | 137 | |
| ✅ | eth-accounts | index.js | 145 | |
| ✅ | eth-accounts | index.js | 151 | |
| ✅ | eth-accounts | index.js | 158 | |
| ✅ | eth-accounts | index.js | 165 | |
| ✅ | eth-accounts | index.js | 230 | |
| ❌ | eth-accounts | index.js | 269 | |
| ❌ | eth-accounts | index.js | 336 | |
| ❌ | eth-accounts | index.js | 342 | |
| ❌ | eth-accounts | index.js | 356 | |
| ❌ | eth-accounts | index.js | 361 | |
| ❌ | eth-accounts | index.js | 368 | |
| ❌ | eth-accounts | index.js | 403 | |
| ❌ | eth-accounts | index.js | 408 | |
| ❌ | eth-accounts | index.js | 546 | |
| ✅ | eth-contract | index.js | 59 | #3190 |
| ✅ | eth-contract | index.js | 74 | #3365 |
| ✅ | eth-contract | index.js | 345 | #3365 |
| ❓ | eth-contract | index.js | 523 | looks unreachable (#3365) |
| ❓ | eth-contract | index.js | 537 | looks unreachable (#3365) |
| ❓ | eth-contract | index.js | 551 | looks unreachable (#3365) |
| ✅ | eth-contract | index.js | 604/6 | #3365 |
| ✅ | eth-contract | index.js | 650 | #3365 |
| ✅ | eth-contract | index.js | 654 | #3365 |
| ✅ | eth-contract | index.js | 673 | |
| ✅ | eth-contract | index.js | 791 | |
| ✅ | eth-contract | index.js | 862 | #3365 |
| ❓ | eth-contract | index.js | 869 | (looks unreachable #3365) |
| ✅ | eth-contract | index.js | 951 | #3365 |
| ✅ | eth-contract | index.js | 955 | |
| ✅ | eth-ens | ENS.js | 176 | |
| ✅ | eth-ens | ENS.js | 182 | |
| ✅ | eth-iban | index.js | 105 | |
| ✅ | eth-iban | ENS.js | 131 | |
| ❌ | providers-http | index.js | 107 | |
| ❌ | providers-http | index.js | 117 | |
| ❌ | providers-http | index.js | 124 | |
| ❌ | providers-ipc | index.js | 151 | |
| ❌ | providers-ipc | index.js | 191 | |
| ❌ | providers-ipc | index.js | 227 | |
| ❌ | providers-ipc | index.js | 251 | |
| 🆗 | providers-ws | index.js| 62 | Removed in #3190 |
| 🆗 | providers-ws | index.js| 203 | Different error in #3190 |
| 🆗 | providers-ws | index.js| 240 | Removed in #3190 |
| 🆗 | providers-ws | index.js| 255 | Removed in #3190 |
| ✅ | providers-ws | index.js| 278 | #3190 |
| ✅ | providers-ws | index.js| 282 | #3190 |
| 🆗 | providers-ws | index.js| 300 | Removed in #3190 |
| ❌ | utils | index.js | 122 | |
| ❌ | utils | index.js | 170 | |
| ✅ | utils | index.js | 218 | |
| ✅ | utils | index.js | 248 | |
| ✅ | utils | index.js | 280 | |
| ❌ | utils | index.js | 300 | |

[nivida]

@cgewecke Great initiative! Is it planned to create a list of already tested errors here? :)

[cgewecke]

@nivida That's a really good idea - it might be helpful to have both.

[cgewecke]

@nivida I've updated the issue with the survey results.

From this point I could start going through in order of priority and writing tests where possible?
Probably each module would be a single PR to reduce merge conflicts and keep it simple.

Many errors are in this format:

throw new Error('You must provide the json interface of the contract when instantiating a contract object.');

Do you think it would be a good idea to define these in web3-core-helpers' error.js and manage all strings there?

[nivida]

Do you think it would be a good idea to define these in web3-core-helpers' error.js and manage all strings there?

@cgewecke I think it makes the most sense to have an error message in the web3-core-helpers package if they are used multiple times. But it will improve the overall error situation if we start to have them at one place for example if we would like to add web3.js error codes do we not to scan the entire code base again. So let us move them as proposed from you in an iterative sense to the web3-core-helpers package and feel free to improve the structure of the web3-core-helpers package if required. 💪

[nivida]

| Nr. | Cov | Package | File | Line | PR |
| ------- | ---------- | ------- | ------- | ------- | ------ |
| 1 | ❓ | providers-ws | index.js| 62 | Removed in #3190 |
| 2 | ❓ | providers-ws | index.js| 203 | Different error in #3190 |
| 3 | ❓ | providers-ws | index.js| 240 | Removed in #3190? |
| 4 | ❓ | providers-ws | index.js| 255 | Removed in #3190? |
| 5 | ❓ | providers-ws | index.js| 300 | Removed in #3190 |

1. This error does no longer exist because Ws is always defined
2. This error got adjusted for #3190 in #3314
3. Because we do now have only one timeout and not two as before (per chunk or/and per request) does the invalid response error there no longer exist but does throw the correct ConnectionTimeout error.
4. This error does get handled correctly now in the _onClose listener.
5. The eventemitter3 dependency does handle this here.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions

[cgewecke]

Leaving open because table may be useful....

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.