Skip to content

Commit

Permalink
feat: consistent stack trace and only export ErrorLogger
Browse files Browse the repository at this point in the history
  • Loading branch information
@gabrocheleau
gabrocheleau committed Feb 21, 2022
1 parent 0489ec8 commit fd78bb9
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 42 deletions.
53 changes: 28 additions & 25 deletions packages/util/src/errors.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export enum ErrorCode {
enum ErrorCode {
INVALID_PARAM = 'INVALID_PARAM',
UNKNOWN_ERROR = 'UNKNOWN_ERROR',
}
Expand All @@ -16,7 +16,7 @@ interface UnknownError extends GeneralError<ErrorCode.UNKNOWN_ERROR> {
}

// Convert an ErrorCode into its Typed Error
export type CodedGeneralError<T> = T extends ErrorCode.INVALID_PARAM
type CodedGeneralError<T> = T extends ErrorCode.INVALID_PARAM
? InvalidParamError
: T extends ErrorCode.UNKNOWN_ERROR
? UnknownError
Expand Down Expand Up @@ -44,47 +44,50 @@ export class ErrorLogger {
static errors = ErrorCode

makeError<T>(codedError: CodedGeneralError<T>): Error {
let { message } = codedError
const { code, message, ...params } = codedError
const messageDetails: Array<string> = []

if (isInvalidParamError(codedError) && typeof codedError.param !== 'undefined') {
if (isInvalidParamError(codedError) && typeof params.param !== 'undefined') {
messageDetails.push(`Invalid param=${codedError.param}`)
}

if (isUnknownError(codedError)) {
Object.keys(codedError)
.filter((key) => ['message', 'code', 'stack'].includes(key))
.forEach((key) => {
const value = codedError[key]
try {
messageDetails.push(key + '=' + JSON.stringify(value))
} catch {
messageDetails.push(key + '=' + JSON.stringify(codedError[key].toString()))
}
})
Object.keys(params).forEach((key) => {
const value = codedError[key]
try {
messageDetails.push(key + '=' + JSON.stringify(value))
} catch {
messageDetails.push(key + '=' + JSON.stringify(codedError[key].toString()))
}
})
}

messageDetails.push(`code=${codedError.code}`)
messageDetails.push(`code=${code}`)

let errorMessage = message ?? ''

if (messageDetails.length) {
message += ' | Details: ' + messageDetails.join(', ')
errorMessage += ' | Details: ' + messageDetails.join(', ')
}

const error = new Error(message) as CodedGeneralError<T>
const error = new Error(errorMessage) as CodedGeneralError<T>
error.code = codedError.code

Object.keys(codedError)
.filter((key) => key !== 'message' && key !== 'code')
.forEach((key) => {
const typedKey = key as keyof typeof codedError
error[typedKey] = codedError[typedKey]
})
Object.keys(params).forEach((key) => {
const typedKey = key as keyof typeof codedError
error[typedKey] = codedError[typedKey]
})

Error.captureStackTrace(error, this.throwError)

throw error
}

throwError<T extends ErrorCode>(code?: T, params?: Omit<CodedGeneralError<T>, 'code'>): never {
throw this.makeError({
throwError<T extends ErrorCode>(
code?: T,
params?: Omit<CodedGeneralError<T>, 'code' | 'stack'>
): void {
this.makeError({
code: code ?? ErrorCode.UNKNOWN_ERROR,
...params,
} as CodedGeneralError<T>)
Expand Down
18 changes: 9 additions & 9 deletions packages/util/test/errors.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import tape from 'tape'
import { ErrorCode, ErrorLogger } from '../src/errors'
import { ErrorLogger } from '../src/errors'

tape('ErrorLogger', (t) => {
const errorLog = new ErrorLogger()
Expand All @@ -13,14 +13,14 @@ tape('ErrorLogger', (t) => {
error = e
}

st.equal(error.code, ErrorCode.UNKNOWN_ERROR)
st.equal(error.code, ErrorLogger.errors.UNKNOWN_ERROR)
st.end()
}),
t.test('should populate an error with UNKNOWN_ERROR code with all provided params', (st) => {
let error: any

try {
errorLog.throwError(ErrorCode.UNKNOWN_ERROR, {
errorLog.throwError(ErrorLogger.errors.UNKNOWN_ERROR, {
errorInfo1: 'Information on the error',
errorInfo2: 'More information on the error',
})
Expand All @@ -36,22 +36,22 @@ tape('ErrorLogger', (t) => {
let error: any

try {
errorLog.throwError(ErrorCode.UNKNOWN_ERROR, {
errorLog.throwError(ErrorLogger.errors.UNKNOWN_ERROR, {
errorInfo1: 'Information on the error',
errorInfo2: 'More information on the error',
})
} catch (e) {
error = e
}

st.equal(error.code, ErrorCode.UNKNOWN_ERROR)
st.equal(error.code, ErrorLogger.errors.UNKNOWN_ERROR)
st.end()
}),
t.test('should add all error params to error message details', (st) => {
let error: any

try {
errorLog.throwError(ErrorCode.UNKNOWN_ERROR, {
errorLog.throwError(ErrorLogger.errors.UNKNOWN_ERROR, {
errorInfo1: 'Information on the error',
errorInfo2: 'More information on the error',
})
Expand All @@ -69,7 +69,7 @@ tape('ErrorLogger', (t) => {
let error: any

try {
errorLog.throwError(ErrorCode.UNKNOWN_ERROR, {
errorLog.throwError(ErrorLogger.errors.UNKNOWN_ERROR, {
message: 'Error Message',
errorInfo1: 'Information on the error',
errorInfo2: 'More information on the error',
Expand All @@ -88,7 +88,7 @@ tape('ErrorLogger', (t) => {
let error: any

try {
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
param: 'difficulty',
})
} catch (e) {
Expand All @@ -102,7 +102,7 @@ tape('ErrorLogger', (t) => {
let error: any

try {
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
message: 'Gas limit higher than maximum',
param: 'gasLimit',
})
Expand Down
12 changes: 6 additions & 6 deletions packages/vm/src/runBlock.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { debug as createDebugLogger } from 'debug'
import { BaseTrie as Trie } from 'merkle-patricia-tree'
import { Account, Address, BN, errorLog, ErrorCode, intToBuffer, rlp } from 'ethereumjs-util'
import { Account, Address, BN, errorLog, ErrorLogger, intToBuffer, rlp } from 'ethereumjs-util'
import { Block } from '@ethereumjs/block'
import { ConsensusType } from '@ethereumjs/common'
import VM from './index'
Expand Down Expand Up @@ -202,7 +202,7 @@ export default async function runBlock(this: VM, opts: RunBlockOpts): Promise<Ru
)
}
const message = _errorMsg('invalid receiptTrie', this, block)
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
message,
param: 'receiptTrie',
})
Expand All @@ -216,7 +216,7 @@ export default async function runBlock(this: VM, opts: RunBlockOpts): Promise<Ru
)
}
const message = _errorMsg('invalid bloom', this, block)
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
message,
param: 'bloom',
})
Expand All @@ -226,7 +226,7 @@ export default async function runBlock(this: VM, opts: RunBlockOpts): Promise<Ru
debug(`Invalid gasUsed received=${result.gasUsed} expected=${block.header.gasUsed}`)
}
const message = _errorMsg('invalid gasUsed', this, block)
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
message,
param: 'gasUsed',
})
Expand All @@ -240,7 +240,7 @@ export default async function runBlock(this: VM, opts: RunBlockOpts): Promise<Ru
)
}
const message = _errorMsg('invalid block stateRoot', this, block)
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
message,
param: 'stateRoot',
})
Expand Down Expand Up @@ -290,7 +290,7 @@ async function applyBlock(this: VM, block: Block, opts: RunBlockOpts) {
if (!opts.skipBlockValidation) {
if (block.header.gasLimit.gte(new BN('8000000000000000', 16))) {
const message = _errorMsg('Invalid block with gas limit greater than (2^63 - 1)', this, block)
errorLog.throwError(ErrorCode.INVALID_PARAM, {
errorLog.throwError(ErrorLogger.errors.INVALID_PARAM, {
message,
param: 'gasLimit',
})
Expand Down
4 changes: 2 additions & 2 deletions packages/vm/tests/api/runBlock.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import tape from 'tape'
import { Address, BN, rlp, KECCAK256_RLP, Account, ErrorCode } from 'ethereumjs-util'
import { Address, BN, rlp, KECCAK256_RLP, Account, ErrorLogger } from 'ethereumjs-util'
import Common, { Chain, Hardfork } from '@ethereumjs/common'
import { Block } from '@ethereumjs/block'
import {
Expand Down Expand Up @@ -212,7 +212,7 @@ tape('runBlock() -> API parameter usage/data errors', async (t) => {
await vm
.runBlock({ block })
.then(() => t.fail('should have returned error'))
.catch((e) => t.ok(e.code === ErrorCode.INVALID_PARAM && e.param === 'gasLimit'))
.catch((e) => t.ok(e.code === ErrorLogger.errors.INVALID_PARAM && e.param === 'gasLimit'))
})

t.test('should fail when block validation fails', async (t) => {
Expand Down

1 comment on commit fd78bb9

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: fd78bb9 Previous: f4f34c8 Ratio
Block 9422906 9699 ops/sec (±8.26%) 20717 ops/sec (±2.02%) 2.14
Block 9422910 8888 ops/sec (±12.96%) 19995 ops/sec (±2.06%) 2.25

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.