Page MenuHomePhabricator

[chronik-client] Improve websocket failover proxy
ClosedPublic

Authored by bytesofman on Nov 10 2023, 13:39.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCf5df71b30758: [chronik-client] Improve websocket failover proxy
Summary

The implementation of websocket in failover proxy causes the app to open multiple websocket connections. This is an async operation. There is no guarantee that the index that comes first will be the websocket that opens first. Using for loops with async functions, we need to await to make sure that we know what we are getting. We also need to await this async method. Currently it is called in the constructor where await is not supported. Move this call to initialization.

Fix resolved to resolve typo.

Test Plan

Before this diff, npm test will pass but also hang indefinitely. After this diff, npm test resolves.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
chronik-client-ws-cycle-patch
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25585
Build 50751: Build Diffchronik-client-tests
Build 50750: arc lint + arc unit

Event Timeline

better code commenting, variable naming

modules/chronik-client/failoverProxy.ts
245

remove unused variable

modules/chronik-client/index.ts
5

this order switch is done automatically by prettier when I save the file

modules/chronik-client/test/test.ts
3

ordering of imports done automatically

Fabien requested changes to this revision.Nov 10 2023, 15:26
Fabien added a subscriber: Fabien.
Fabien added inline comments.
modules/chronik-client/failoverProxy.ts
224 ↗(On Diff #43031)

If none of the endpoints is available you just created an infinite loop

This revision now requires changes to proceed.Nov 10 2023, 15:26
bytesofman marked an inline comment as done.

No infinite loop, add test to confirm expected error

Fabien requested changes to this revision.Nov 15 2023, 08:31
Fabien added inline comments.
modules/chronik-client/failoverProxy.ts
275 ↗(On Diff #43082)

This is the clear sign that you don't need the while loop

This revision now requires changes to proceed.Nov 15 2023, 08:31
Fabien requested changes to this revision.Nov 16 2023, 14:18

Don't special case a loop iteration

modules/chronik-client/failoverProxy.ts
271–275 ↗(On Diff #43125)
This revision now requires changes to proceed.Nov 16 2023, 14:18

remove special case for handling hangover from while loop, define ws before adding it to wsEndpoint, add error handling to ws tests

This revision is now accepted and ready to land.Nov 16 2023, 15:17