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.

Sign up for GitHub

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

fix[bs]: disambiguate generic submission errors #1051

Merged
merged 4 commits into from
Jun 15, 2021
Merged

fix[bs]: disambiguate generic submission errors #1051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d76399
fix[bs]: disambiguate generic submission errors
annieke Jun 10, 2021
b9dcfed
add cases for errors
annieke Jun 10, 2021
3d6740a
separate out errors from transaction reverts with reasons
annieke Jun 11, 2021
7e25256
remove extraneous errors
annieke Jun 15, 2021
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
5 changes: 5 additions & 0 deletions .changeset/selfish-dryers-teach.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@eth-optimism/batch-submitter': patch
---

Add status to generic error log to disambiguate errors
29 changes: 24 additions & 5 deletions packages/batch-submitter/src/batch-submitter/batch-submitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,30 @@ export abstract class BatchSubmitter {
gasRetryIncrement: this.gasRetryIncrement,
}

const receipt = await BatchSubmitter.getReceiptWithResubmission(
txFunc,
resubmissionConfig,
this.logger
)
let receipt: TransactionReceipt
try {
receipt = await BatchSubmitter.getReceiptWithResubmission(
txFunc,
resubmissionConfig,
this.logger
)
} catch (err) {
if (err.reason) {
this.logger.error(`Transaction invalid: ${err.reason}, aborting`, {
message: err.toString(),
stack: err.stack,
code: err.code,
})
return
}

this.logger.error('Encountered error at submission, aborting', {
message: err.toString(),
stack: err.stack,
code: err.code,
})
return
}

this.logger.info('Received transaction receipt', { receipt })
this.logger.info(successMessage)
Expand Down
3 changes: 1 addition & 2 deletions packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,7 @@ export class TransactionBatchSubmitter extends BatchSubmitter {
// Fix our batches if we are configured to. TODO: Remove this.
batch = await this._fixBatch(batch)
if (!(await this._validateBatch(batch))) {
this.logger.error('Batch is malformed! Cannot submit next batch!')
throw new Error('Batch is malformed! Cannot submit next batch!')
return
}
let sequencerBatchParams = await this._getSequencerBatchParams(
startBlock,
Expand Down
28 changes: 23 additions & 5 deletions packages/batch-submitter/src/exec/run-batch-submitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,11 +441,29 @@ export const run = async () => {
try {
await func()
} catch (err) {
logger.error('Error submitting batch', {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the status should be added to the object instead of being placed in string to allow for searching on subkeys more easily. I'm not an expert on sentry but my intuition is that you can get all errors with an exact match of Error submitting batch and then filter based on the values in the object and then also more easily create things like pie charts that show the percentages of the different statuses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

might be what we have to do since it seems like the errors both triggered as we submit the transaction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i left the status in one log for server errors so we can distinguish between the different ones, since sentry categorizes these errors by log line and doesn't have a way to filter by the rest of the error object

message: err.toString(),
stack: err.stack,
code: err.code,
})
switch (err.code) {
case 'SERVER_ERROR':
logger.error(`Encountered server error with status ${err.status}`, {
message: err.toString(),
stack: err.stack,
code: err.code,
})
break
case 'NETWORK_ERROR':
logger.error('Could not detect network', {
message: err.toString(),
stack: err.stack,
code: err.code,
})
break
default:
logger.error('Unhandled exception during batch submission', {
message: err.toString(),
stack: err.stack,
code: err.code,
})
break
}
logger.info('Retrying...')
}
// Sleep
Expand Down