Use try-catch for sending Functions requests #195
Conversation
KuphJr
requested review from
justinkaseman,
zeuslawyer,
aelmanaa and
pinebit
| const autoConsumerContractFactory = await ethers.getContractFactory("AutomatedFunctionsConsumer") | ||
| const autoConsumerContract = await autoConsumerContractFactory.attach(taskArgs.contract) | ||
|
|
||
| const checkUpkeep = await autoConsumerContract.checkUpkeep(performData) |
There was a problem hiding this comment.
Possibly a nit: Given that the goal is to make sure that we do not re run automation when the sub has no balance could we not just
- performUpkeep anyway and handle a reviert if checkUpkeep returns false (this logic is now in performupkeep)
- console log the error (with string or without) if the functions request fails
Do we need to call checkUpkeep separately here?
There was a problem hiding this comment.
I updated the console.log message slightly, although I do think how it is currently implemented is how I intended.
There was a problem hiding this comment.
sorry @KuphJr i commented over too many lines! I was wondering whether line 22 could just go straight to performUpkeep (and rely on its internal checkupkeep call).
| const checkUpkeep = await autoConsumerContract.checkUpkeep(performData) | ||
| if (!checkUpkeep.upkeepNeeded) { | ||
| console.log("\ncheckUpkeep returned false. Upkeep was not performed.") | ||
| return |
There was a problem hiding this comment.
This might be a little annoying if somebody wants to force a performUpkeep but now needs to wait for s_updateInterval to pass. How is this command expected to be used? Maybe put calling checkUpkeep under a flag?
There was a problem hiding this comment.
Noted. I can just remove this check and allow performUpkeep to revert if checkUpkeep returns false.
performUpkeepto revert if the Functions subscription lacked funds. This meant the last upkeep time was never updated, so Automation kept retrying the upkeep and could spend all the funds in the Automation subscription.try-catchprevents Automation from attempting repeated failed upkeeps since the last upkeep time is always updated even if the Functions request fails instead of the entire transaction reverting