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

monorepo: Normalize exception values in error handling #3700

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7d5d4ee
Edit eslint typescript rules
ScottyPoi Sep 24, 2024
3bd3d67
block: remove unused e variable
ScottyPoi Sep 24, 2024
feb647b
block/test: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
7e99bb6
blockchain: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
3923362
client: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
f3543fc
common: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
cd53d55
devp2p: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
cf4f8bc
evm: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
75ffbbf
rlp: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
a38c697
statemanager: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
13ea657
trie: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
9df3660
vm: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
e77fae1
wallet: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
92b33d1
tx: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
d7807e3
util: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
9678ae4
verkle: replace (e: any) pattern with Error type check
ScottyPoi Sep 24, 2024
96d13a9
fix rule format
ScottyPoi Sep 24, 2024
48f2d90
evm: revert change for custom error object
ScottyPoi Sep 24, 2024
220d1a2
remove pattern where unnecessary
ScottyPoi Sep 24, 2024
2a18d4f
revert change to helper function
ScottyPoi Sep 24, 2024
f9048e0
client: revert where unnecessary
ScottyPoi Sep 25, 2024
12df664
Merge branch 'master' into any-error
holgerd77 Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/eslint.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ module.exports = {
'error',
{ argsIgnorePattern: '^_', varsIgnorePattern: '^_' },
],
'@typescript-eslint/no-redeclare': ['error'],
'@typescript-eslint/no-redeclare': 0,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we have to set this down, when reading on no-redeclare we are not redeclaring anyhting but only reassiging, which is totally norma, right?

I did a quick test re-adding the original rule setup and runnpm run lint from root, still seems to work.

'no-ex-assign': 0,
Copy link
Member

Choose a reason for hiding this comment

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

Ok, this rule no-ex-assign seems out of question (as discussed)

'@typescript-eslint/no-unnecessary-condition': 'off',
'@typescript-eslint/prefer-nullish-coalescing': 'error',
'@typescript-eslint/restrict-plus-operands': 'off',
Expand Down
3 changes: 2 additions & 1 deletion config/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"strict": true,
"target": "es2020",
"lib": ["ES2020", "DOM"],
"skipLibCheck": true
"skipLibCheck": true,
"useUnknownInCatchVariables": false
Copy link
Member

Choose a reason for hiding this comment

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

Ah, this needs to be the otheway around: we want to have unknown there, atm this now goes to any:

grafik

So and this is the version of when removing the new assignment with e still being any:

grafik

So no underlying of e.message (what we would want. any is just saying: use whatever you want, I do not care! 🙂

With unknown is the more strict version - indicating that for e we just don't (and can't) know the type.

Ah, just seeing that a plain use in new Error() is then not directly possible. 🤔

grafik grafik

But for any we still have typed as any. Also not optimal:

grafik

Hmm. Let me give this some thought, input welcome from the team.

}
}
4 changes: 2 additions & 2 deletions packages/block/src/block/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,13 +607,13 @@ export class Block {
let hash = ''
try {
hash = bytesToHex(this.hash())
} catch (e: any) {
} catch {
hash = 'error'
}
let hf = ''
try {
hf = this.common.hardfork()
} catch (e: any) {
} catch {
hf = 'error'
}
let errorStr = `block number=${this.header.number} hash=${hash} `
Expand Down
4 changes: 2 additions & 2 deletions packages/block/src/header/header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -806,13 +806,13 @@ export class BlockHeader {
let hash = ''
try {
hash = bytesToHex(this.hash())
} catch (e: any) {
} catch {
hash = 'error'
}
let hf = ''
try {
hf = this.common.hardfork()
} catch (e: any) {
} catch {
hf = 'error'
}
let errorStr = `block header number=${this.number} hash=${hash} `
Expand Down
30 changes: 24 additions & 6 deletions packages/block/test/block.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ describe('[Block]: block functions', () => {
try {
createBlockFromRLP(blockRlp, { common })
assert.ok(true, 'should pass')
} catch (error: any) {
} catch (error) {
if (!(error instanceof Error)) {
error = new Error(error)
}
assert.fail('should not throw')
}
})
Expand All @@ -165,7 +168,10 @@ describe('[Block]: block functions', () => {
try {
createBlockFromRPC(testdataFromRPCGoerliData, [], { common })
assert.ok(true, 'does not throw')
} catch (error: any) {
} catch (error) {
if (!(error instanceof Error)) {
error = new Error(error)
}
assert.fail('error thrown')
}
})
Expand All @@ -184,7 +190,10 @@ describe('[Block]: block functions', () => {
try {
await block.validateData()
assert.fail('should throw')
} catch (error: any) {
} catch (error) {
if (!(error instanceof Error)) {
error = new Error(error)
}
assert.ok((error.message as string).includes('invalid transaction trie'))
}
})
Expand All @@ -205,7 +214,10 @@ describe('[Block]: block functions', () => {
try {
await block.validateData()
assert.fail('should throw')
} catch (error: any) {
} catch (error) {
if (!(error instanceof Error)) {
error = new Error(error)
}
assert.ok((error.message as string).includes('unsigned'))
}
})
Expand Down Expand Up @@ -237,7 +249,10 @@ describe('[Block]: block functions', () => {
try {
await block.validateData()
assert.fail('should throw')
} catch (error: any) {
} catch (error) {
if (!(error instanceof Error)) {
error = new Error(error)
}
assert.ok((error.message as string).includes('invalid uncle hash'))
}
})
Expand All @@ -260,7 +275,10 @@ describe('[Block]: block functions', () => {
try {
await fn
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok((e.message as string).includes(errorMsg))
}
}
Expand Down
40 changes: 32 additions & 8 deletions packages/block/test/eip1559block.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ describe('EIP1559 tests', () => {
},
)
assert.fail('should throw when baseFeePerGas is not set to initial base fee')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
const expectedError = 'Initial EIP1559 block does not have initial base fee'
assert.ok(
e.message.includes(expectedError),
Expand All @@ -96,7 +99,10 @@ describe('EIP1559 tests', () => {
)
;(header as any).baseFeePerGas = undefined
await (header as any)._genericFormatValidation()
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
const expectedError = 'EIP1559 block has no base fee field'
assert.ok(
e.message.includes(expectedError),
Expand Down Expand Up @@ -141,7 +147,10 @@ describe('EIP1559 tests', () => {
},
)
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok(e.message.includes('base fee'), 'should throw on wrong initial base fee')
}
})
Expand Down Expand Up @@ -199,7 +208,10 @@ describe('EIP1559 tests', () => {
},
)
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok(e.message.includes('too much gas used'), 'should throw when elasticity is exceeded')
}
})
Expand Down Expand Up @@ -325,7 +337,10 @@ describe('EIP1559 tests', () => {
try {
header.validateGasLimit(genesis.header)
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok(
e.message.includes('gas limit increased too much'),
'should throw if gas limit is increased too much (HF transition block)',
Expand All @@ -349,7 +364,10 @@ describe('EIP1559 tests', () => {
try {
header.validateGasLimit(block1.header)
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok(
e.message.includes('gas limit increased too much'),
'should throw if gas limit is increased too much (post-HF transition block)',
Expand All @@ -375,7 +393,10 @@ describe('EIP1559 tests', () => {
try {
header.validateGasLimit(genesis.header)
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok(
e.message.includes('gas limit decreased too much'),
'should throw if gas limit is decreased too much (HF transition block)',
Expand All @@ -399,7 +420,10 @@ describe('EIP1559 tests', () => {
try {
header.validateGasLimit(block1.header)
assert.fail('should throw')
} catch (e: any) {
} catch (e) {
if (!(e instanceof Error)) {
e = new Error(e)
}
assert.ok(
e.message.includes('gas limit decreased too much'),
'should throw if gas limit is decreased too much (post-HF transition block)',
Expand Down
5 changes: 4 additions & 1 deletion packages/block/test/from-rpc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,10 @@ describe('[fromJSONRPCProvider]', () => {
try {
await createBlockFromJSONRPCProvider(provider, bytesToHex(randomBytes(32)), {})
assert.fail('should throw')
} catch (err: any) {
} catch (err) {
if (!(err instanceof Error)) {
err = new Error(err)
}
assert.ok(
err.message.includes('No block data returned from provider'),
'returned correct error message',
Expand Down
Loading
Loading