Skip to content

feat: add ThreadBoundaryError - #5499

Merged
wemeetagain merged 3 commits into
dapplion/network-threadfrom
tuyen/network_thread_req_resp_e2e
May 17, 2023
Merged

feat: add ThreadBoundaryError#5499
wemeetagain merged 3 commits into
dapplion/network-threadfrom
tuyen/network_thread_req_resp_e2e

Conversation

@twoeths

@twoeths twoeths commented May 17, 2023

Copy link
Copy Markdown
Member

Motivation

  • Failed reqresp e2e tests because Error objects cannot be passed through thread boundary correctly

Description

  • Add ThreadBoundaryError that can be passed through thread boundaries
    • for errors that's clonnable, error is null and object is the result of toObject() method
    • for other errors, just pass object as null. They will be deserialized into the generic Error object if passing through thread boundaries
export type ThreadBoundaryError = {error: null; object: LodestarErrorObject} | {error: Error; object: null};
  • For errors that we want to pass through thread boundaries, they need to

    • implement a newly created ClonableLodestarError class which includes class name there in toObject method
    • implement static fromObject method
    • for now I see only RequestError and ResponseError need this
  • For other errors (don't see the need for now) it'll be deserialized into the generic Error object if we pass them through thread boundaries

@twoeths
twoeths marked this pull request as ready for review May 17, 2023 05:37
@twoeths
twoeths requested a review from a team as a code owner May 17, 2023 05:37
@wemeetagain
wemeetagain merged commit fe91dbd into dapplion/network-thread May 17, 2023
@wemeetagain
wemeetagain deleted the tuyen/network_thread_req_resp_e2e branch May 17, 2023 16:02
wemeetagain added a commit that referenced this pull request May 17, 2023
* Implement libp2p network thread

* Temp enable worker

* Fix worker path

* Make discv5 options serializable

* Move reqResp bridge events to separate EventEmitter

* Wire gossip subscribe events

* Wire events to worker

* Send clonable data only

* Fix test types

* Represent PeerId in main thread as string

* Fix unit tests

* Close network core

* Ensure network worker data is safe to serialized to worker

* fix: NetworkEvent.gossipMessageValidationResult should be from main to worker

* fix: remove my test file

* small cleanups

* more simplification

* fix: network e2e tests

* Fix gossipsub e2e test

* feat: add ThreadBoundaryError (#5499)

* feat: add ThreadBoundaryError

* Remove ClonableLodestarError

* Fix unit test

---------

Co-authored-by: Cayman <caymannava@gmail.com>

---------

Co-authored-by: Tuyen Nguyen <vutuyen2636@gmail.com>
Co-authored-by: Cayman <caymannava@gmail.com>
Co-authored-by: tuyennhv <tuyen@chainsafe.io>
nazarhussain added a commit that referenced this pull request Aug 6, 2026
consensus-specs #5498/#5499 fix the double/skipped on_fast_confirmation
runs; unskip on the next spec-tests release.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nazarhussain added a commit that referenced this pull request Aug 6, 2026
consensus-specs #5498/#5499 fix the double/skipped on_fast_confirmation
runs; unskip on the next spec-tests release.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nazarhussain added a commit that referenced this pull request Aug 7, 2026
consensus-specs #5498/#5499 fix the double/skipped on_fast_confirmation
runs; unskip on the next spec-tests release.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.

2 participants