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.

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

Stream fallback: error handler #12173

Merged

Conversation

infiloop2
Copy link
Contributor

AUTO-9004

amirylm and others added 30 commits February 15, 2024 20:55
* return timeout for retry interval

* identify retryTimeout + placeholder for err handler

* err codes

* comments

* added function and tests for handling err code
* err codes for conditionals

* unit tests for conditional
…handler

Call check error handler within mercury
amirylm and others added 12 commits February 23, 2024 16:36
- server timeout
- unauthorized
- bad req
- internal server err
- not found
…er-integration-test

Error handler integration test
* 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 <amirylm.dev@gmail.com>
select {
case <-reqTimeoutCtx.Done():
// Request Timed out, return timeout error
c.lggr.Errorf("at timestamp %s upkeep %s, streams lookup v0.3 timed out", streamsLookup.Time.String(), streamsLookup.UpkeepId.String())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Warning might be a better level in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

imo it's rare enough to be an error, and if this happens regularly then i would want NOPs to flag

Copy link
Contributor

I see that you haven't updated any README files. Would it make sense to do so?

Copy link
Contributor

@anirudhwarrier anirudhwarrier left a comment

Choose a reason for hiding this comment

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

Final QA check done with build 0c028b4303828af8089274f9af626fadd6a32ca5
LGTM

@infiloop2 infiloop2 merged commit 9659252 into release-2.9.1-automation-20240220 Feb 27, 2024
114 checks passed
@infiloop2 infiloop2 deleted the auto-9004-stream-err-handler branch February 27, 2024 11:27
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 <amirylm.dev@gmail.com>

* Empty Commit

---------

Co-authored-by: Amir Y <amirylm.dev@gmail.com>
Co-authored-by: Lei <lei.shi@smartcontract.com>
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 <amirylm.dev@gmail.com>

* Empty Commit

---------

Co-authored-by: Amir Y <amirylm.dev@gmail.com>
Co-authored-by: Lei <lei.shi@smartcontract.com>

* Empty Commit

---------

Co-authored-by: Amir Y <amirylm.dev@gmail.com>
Co-authored-by: Lei <lei.shi@smartcontract.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants