Conversation
gvelez17
changed the title
|
Hey @stephhuynh18 this is the one you looked at before, its not addressing the infura api limits but i didn't want to discard it since i think we do want an alert on eth transaction errors |
| } catch (err: any) { | ||
| logger.err(err) | ||
| const { code } = err | ||
| Metrics.count(METRIC_NAMES.ETH_REQUEST_ERROR_TOTAL, 1, {'error_code': code}) |
There was a problem hiding this comment.
There is a case where this "should" fail. Ex. First attempt is successful but times out before it can return success. Second attempt fails because the first attempt was successful (the error is nonce expired). Instead of throwing the error we check if the first attempt was successful.
If this count disregards the above scenario then you can also disregard this comment
There was a problem hiding this comment.
ok yeah that would change the behavior aside from metrics - i guess i will defer this or icebox for now, bc i think changing the error behavior of cas is out of scope from measuring the things
The code should have something to do with the reason but not sure if it will sort these...
| const txData = await this._buildTransactionRequest(rootCid) | ||
| const txResponses: Array<TransactionResponse> = [] | ||
|
|
||
| Metrics.count(METRIC_NAMES.ETH_REQUEST_TOTAL, 1) |
There was a problem hiding this comment.
What does this metric represent?
There was a problem hiding this comment.
yeah its just for comparing to the error total - its like how many times did we make an eth transaction request, vs how many times it errored
it might also be the same as the number of anchors but unless we have knowledge of the code we don't know that for sure
There was a problem hiding this comment.
Yes currently this should corresponding to the number of anchors. Do you want this to include the number of retries? If so then you should move it into the try block after _trySendTransaction.
There was a problem hiding this comment.
i don't think i want to move it to include retries, its just for the purpose of comparing to errors. we could remove it entirely and just count the errors maybe
There was a problem hiding this comment.
i don't think i want to move it to include retries, its just for the purpose of comparing to errors. we could remove it entirely and just count the errors maybe
I think that makes sense
2 participants
Count Eth transactions and errors - #PLAT-1511
Description
We probably want to be alerted on eth insufficient funds and other eth transaction errors. Count the errors and also the totals. (we expect the totals to match the anchors)
How Has This Been Tested?
Definition of Done
Before submitting this PR, please make sure:
References:
Please list relevant documentation (e.g. tech specs, articles, related work etc.) relevant to this change, and note if the documentation has been updated.