add checkErrorHandler #12037
Merged
add checkErrorHandler #12037
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
shileiwill
force-pushed
the
AUTO-9015-new
branch
from
4c2612d to
52cc826
Compare
shileiwill
force-pushed
the
AUTO-9015-new
branch
from
52cc826 to
1649d53
Compare
RensR
reviewed
Feb 15, 2024
| @@ -116,4 +116,12 @@ contract VerifiableLoadLogTriggerUpkeep is VerifiableLoadBase, StreamsLookupComp | |||
| bytes memory performData = abi.encode(values, extraData); | |||
| return (true, performData); | |||
| } | |||
|
|
|||
| function checkErrorHandler( | |||
There was a problem hiding this comment.
These are Automation test files correct? Can we move them into the Automation folder? The top level test folder is deprecated and everything should be moving out to project folders
There was a problem hiding this comment.
Yes, this is a test contract used by automation, will do the migration, tracked in https://smartcontract-it.atlassian.net/browse/AUTO-9028
infiloop2
reviewed
Feb 15, 2024
| @@ -17,4 +17,17 @@ interface StreamsLookupCompatibleInterface { | |||
| bytes[] memory values, | |||
| bytes memory extraData | |||
| ) external view returns (bool upkeepNeeded, bytes memory performData); | |||
|
|
|||
| /** | |||
| * @dev this is a new, optional function in v2.1. It is meant to surface streams lookup errors. | |||
There was a problem hiding this comment.
nit.
- use @notice as above function
this is a new, optional function in v2.1.->this is a new, optional function in streams lookup. (Streams lookup is independent of v2.1 and will be supported in future versions)
infiloop2
reviewed
Feb 15, 2024
Comment on lines
147
to
154
| function checkErrorHandler( | ||
| uint256 errCode, | ||
| bytes memory extraData | ||
| ) external view override returns (bool upkeepNeeded, bytes memory performData) { | ||
| // dummy function, just return true and extraData | ||
| return (true, extraData); | ||
| } | ||
|
|
There was a problem hiding this comment.
default implementation should be false to keep existing behaviour
return (false, new bytes(0))
Also you might be able to define this within the interface itself without requiring all contracts to change (not sure if that works)
There was a problem hiding this comment.
well, just checked : Functions in interfaces cannot have an implementation.
shileiwill
force-pushed
the
AUTO-9015-new
branch
from
1649d53 to
b3a8054
Compare
|
infiloop2
added a commit
that referenced
this pull request
Feb 27, 2024
* Stop retry flow for error handling (log trigger) (#12026) * return timeout for retry interval * identify retryTimeout + placeholder for err handler * err codes * comments * added function and tests for handling err code * add checkErrorHandler (#12037) * Stop retry flow for error handling (conditional trigger) (#12032) * err codes for conditionals * unit tests for conditional * refactor streams error handler * fix mercury v0.2 request handling * fix mercury 0.3 DoRequest * fix tests * connect check error callback * add todo * add todo * improve comments * polish 0.2 * fix debug.go, refactor eth_call on checkCallback and checkErrorHandler, fix some minor comments (typo and test) * small fixes * calculate retry config for conditionals * rename to clarify function * cleanup pipeline execution errors * fix unit tests for v02_request.go * Fix bug in 0.3 request error code * add state assertion to single feed request tests * add test case for invalid response bytes * add tests for more retryable errors * add tests for retryable -> error conversion * polish 0.2 combining multiple feeds * add more tests for different combination of feed responses * remove unused fields * unit tests for v03 * fix mercury_test.go * minor polishing * thread control: added function that accept a context * use threadCtrl.GoCtx to ensure the timeout is applied with the provided context * added timeout for mercury requests (including retries) * fix lint * set timeout to 10s * use GoCtx within request clients * lint * add more 0.3 tests * set err code to timeout if ctx is done * Finalize stream error codes, polish requests to return consistent nil bytes upon error, use HttpToStreamsErrCode everywhere * add tests for checkErrorHanlder * treat timeout as non retryable error codes * allow empty feed request which returns error code * update test contract with a flag for checkErr result * generate wrappers for LogTriggeredStreamsLookup * handling empty performData case for err handler * test (wip) * fixing contract * waiting for err handler logs (wip) * update contract and generate wrappers * lint * use startBlock instead of 1 * add missing arg * check multiple responses: - server timeout - unauthorized - bad req - internal server err - not found * fix test * cleanup * set timeout in http client * Add timeout test for streams (#12170) * Add timeout test * fix DoRequest() to consider ctx (client v0.3) * make sure we timeout in time * align thread control test * hacky context timeout * trying to wait for timeout with child context * push hack * update * another fix * fix * use ctx background * add log * Use new ctx to implement timeout * fix test * add v0.2 test --------- Co-authored-by: amirylm <[email protected]> * Empty Commit --------- Co-authored-by: Amir Y <[email protected]> Co-authored-by: Lei <[email protected]>
infiloop2
added a commit
that referenced
this pull request
Feb 27, 2024
* Stop retry flow for error handling (log trigger) (#12026) * return timeout for retry interval * identify retryTimeout + placeholder for err handler * err codes * comments * added function and tests for handling err code * add checkErrorHandler (#12037) * Stop retry flow for error handling (conditional trigger) (#12032) * err codes for conditionals * unit tests for conditional * refactor streams error handler * fix mercury v0.2 request handling * fix mercury 0.3 DoRequest * fix tests * connect check error callback * add todo * add todo * improve comments * polish 0.2 * fix debug.go, refactor eth_call on checkCallback and checkErrorHandler, fix some minor comments (typo and test) * small fixes * calculate retry config for conditionals * rename to clarify function * cleanup pipeline execution errors * fix unit tests for v02_request.go * Fix bug in 0.3 request error code * add state assertion to single feed request tests * add test case for invalid response bytes * add tests for more retryable errors * add tests for retryable -> error conversion * polish 0.2 combining multiple feeds * add more tests for different combination of feed responses * remove unused fields * unit tests for v03 * fix mercury_test.go * minor polishing * thread control: added function that accept a context * use threadCtrl.GoCtx to ensure the timeout is applied with the provided context * added timeout for mercury requests (including retries) * fix lint * set timeout to 10s * use GoCtx within request clients * lint * add more 0.3 tests * set err code to timeout if ctx is done * Finalize stream error codes, polish requests to return consistent nil bytes upon error, use HttpToStreamsErrCode everywhere * add tests for checkErrorHanlder * treat timeout as non retryable error codes * allow empty feed request which returns error code * update test contract with a flag for checkErr result * generate wrappers for LogTriggeredStreamsLookup * handling empty performData case for err handler * test (wip) * fixing contract * waiting for err handler logs (wip) * update contract and generate wrappers * lint * use startBlock instead of 1 * add missing arg * check multiple responses: - server timeout - unauthorized - bad req - internal server err - not found * fix test * cleanup * set timeout in http client * Add timeout test for streams (#12170) * Add timeout test * fix DoRequest() to consider ctx (client v0.3) * make sure we timeout in time * align thread control test * hacky context timeout * trying to wait for timeout with child context * push hack * update * another fix * fix * use ctx background * add log * Use new ctx to implement timeout * fix test * add v0.2 test --------- Co-authored-by: amirylm <[email protected]> * Empty Commit --------- Co-authored-by: Amir Y <[email protected]> Co-authored-by: Lei <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 27, 2024
* Stream fallback: error handler (#12173) * Stop retry flow for error handling (log trigger) (#12026) * return timeout for retry interval * identify retryTimeout + placeholder for err handler * err codes * comments * added function and tests for handling err code * add checkErrorHandler (#12037) * Stop retry flow for error handling (conditional trigger) (#12032) * err codes for conditionals * unit tests for conditional * refactor streams error handler * fix mercury v0.2 request handling * fix mercury 0.3 DoRequest * fix tests * connect check error callback * add todo * add todo * improve comments * polish 0.2 * fix debug.go, refactor eth_call on checkCallback and checkErrorHandler, fix some minor comments (typo and test) * small fixes * calculate retry config for conditionals * rename to clarify function * cleanup pipeline execution errors * fix unit tests for v02_request.go * Fix bug in 0.3 request error code * add state assertion to single feed request tests * add test case for invalid response bytes * add tests for more retryable errors * add tests for retryable -> error conversion * polish 0.2 combining multiple feeds * add more tests for different combination of feed responses * remove unused fields * unit tests for v03 * fix mercury_test.go * minor polishing * thread control: added function that accept a context * use threadCtrl.GoCtx to ensure the timeout is applied with the provided context * added timeout for mercury requests (including retries) * fix lint * set timeout to 10s * use GoCtx within request clients * lint * add more 0.3 tests * set err code to timeout if ctx is done * Finalize stream error codes, polish requests to return consistent nil bytes upon error, use HttpToStreamsErrCode everywhere * add tests for checkErrorHanlder * treat timeout as non retryable error codes * allow empty feed request which returns error code * update test contract with a flag for checkErr result * generate wrappers for LogTriggeredStreamsLookup * handling empty performData case for err handler * test (wip) * fixing contract * waiting for err handler logs (wip) * update contract and generate wrappers * lint * use startBlock instead of 1 * add missing arg * check multiple responses: - server timeout - unauthorized - bad req - internal server err - not found * fix test * cleanup * set timeout in http client * Add timeout test for streams (#12170) * Add timeout test * fix DoRequest() to consider ctx (client v0.3) * make sure we timeout in time * align thread control test * hacky context timeout * trying to wait for timeout with child context * push hack * update * another fix * fix * use ctx background * add log * Use new ctx to implement timeout * fix test * add v0.2 test --------- Co-authored-by: amirylm <[email protected]> * Empty Commit --------- Co-authored-by: Amir Y <[email protected]> Co-authored-by: Lei <[email protected]> * Empty Commit --------- Co-authored-by: Amir Y <[email protected]> Co-authored-by: Lei <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AUTO-9015
Add checkErrorHandler in contracts/src/v0.8/automation/interfaces/StreamsLookupCompatibleInterface.sol
All new contracts can add this checkErrorHandler function so they can have the streams lookup errors.