ws.ping() is not available in the browser WebSocket object. However, ws.send is.
Implement a browser ping by subscribing to the first existing subscription if the ws.ping() method is unavailable.
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Project - Commits
- rABC81f90ded8ad6: [chronik-client] Support ping / pong in browser environment
In nodejs, install this version of chronik-client and connect to a websocket. Subscribe. Add console.log statements to confirm ws.ping() is being used.
Implement this version of chronik-client in cashtab and npm start. Ref D15050. Confirm the browser ping is working.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- chronik-client-ping-mods
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 26081 Build 51735: Build Diff chronik-client-tests · chronik-client-integration-tests Build 51734: arc lint + arc unit
Event Timeline
Comment Actions
nodejs testing
cashtab testing
... I don't have screenshots but I did run it with the ref diff and confirm no ws disconnects and the browser ping method was activated
Comment Actions
Note: would be nice to just ws.send('ping') -- however this would require the correct encoding, which would require adding this to the chronik.proto file. This is probably easy enough to do -- i'm checking with @tobias_ruck on the approach. I'm not really sure if there is any performance difference between 'ping' and subscribing to an already existing subscription. pinging is certainly more elegant.
Comment Actions
Tail of the build log:
48 packages are looking for funding run `npm fund` for details 1 moderate severity vulnerability To address all issues (including breaking changes), run: npm audit fix --force Run `npm audit` for details. > chronik-client@0.10.0 integration-tests > mocha -j1 -r ts-node/register test/integration/*.ts --reporter mocha-junit-reporter --reporter-options mochaFile=test_results/chronik-client-integration-tests-junit.xml --reporter-options testsuitesTitle=Chronik Client Integration Tests --reporter-options rootSuiteTitle=Chronik Client Starting test_runner Test runner started 2023-12-29T21:59:42.547000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20231229_215942/setup_scripts/chronik-client_blockchain_info_0 Setting chronik url to http://127.0.0.1:30001 2023-12-29T21:59:44.340000Z TestFramework (INFO): Step 1: Mine 10 more blocks 2023-12-29T21:59:44.353000Z TestFramework (INFO): Received a stop message, exiting 2023-12-29T21:59:44.404000Z TestFramework (INFO): Stopping nodes 2023-12-29T21:59:45.412000Z TestFramework (INFO): Cleaning up /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20231229_215942/setup_scripts/chronik-client_blockchain_info_0 on exit 2023-12-29T21:59:45.412000Z TestFramework (INFO): Tests successful Running Unit Tests for Test Framework Modules [1msetup_scripts/chronik-client_blockchain_info.py[0m started [1msetup_scripts/chronik-client_blockchain_info.py[0m passed, Duration: 3 s [1m TEST | STATUS | DURATION [0m[0;32msetup_scripts/chronik-client_blockchain_info.py | ✓ Passed | 3 s [0m[1m ALL | ✓ Passed | 3 s (accumulated) [0mRuntime: 3 s -----------------------|---------|----------|---------|---------|----------------------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------------|---------|----------|---------|---------|----------------------------------- All files | 5.36 | 0.63 | 1.47 | 5.34 | chronik-client | 100 | 100 | 100 | 100 | index.ts | 100 | 100 | 100 | 100 | chronik-client/proto | 4.9 | 0.67 | 1.72 | 4.89 | chronik.ts | 5.44 | 0.84 | 1.81 | 5.42 | ...,3978-3985,3990-4027,4031-4036 chronikNode.ts | 4.15 | 0.45 | 1.59 | 4.17 | ...,2682-2729,2740-2804,2808-2813 chronik-client/src | 8.51 | 0 | 0 | 8.46 | ChronikClient.ts | 4.19 | 0 | 0 | 4.24 | 29-159,174-218,296-699 ChronikClientNode.ts | 40 | 100 | 0 | 40 | 23-42 failoverProxy.ts | 3.92 | 0 | 0 | 4.04 | 27-313 hex.ts | 31.57 | 0 | 0 | 33.33 | 30-34,38-42,46-59,63-65 -----------------------|---------|----------|---------|---------|----------------------------------- ##teamcity[blockOpened name='Code Coverage Summary'] ##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='156'] ##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='2909'] ##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='21'] ##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='3286'] ##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='8'] ##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='541'] ##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='154'] ##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='2881'] ##teamcity[blockClosed name='Code Coverage Summary'] Build chronik-client-integration-tests failed with exit code 1
Comment Actions
update: the best way to do this is have chronik send pings on the server side. this is in Tobias queue but not sure when it will get in.
In the meantime, I think it's worth publishing this as
- Improves on what we have now
- Is required to implement latest chronik-client into Cashtab
Comment Actions
Why is the subscribe better than a dummy send ? Does the server disconnect on unknown frames ? If yes this is a bug and should be fixed on the server.
| modules/chronik-client/src/failoverProxy.ts | ||
|---|---|---|
| 231 ↗ | (On Diff #43813) | you doubled the brackets |
Comment Actions
A dummy send requires a dummy type in the chronik.proto file, then running the proto script here to support it. So, it would be best in this case to add the actual ping type with the dummy script.
For example, wsEndpoint.ws.send('ping') throws a wiretype error. I managed to find the hex that is usually sent for ping (couldn't immediately find it just now), but this also throws a wiretype error. I think there's not really a standard for this as some servers do different things, for example the blockchain.com websocket uses {op: 'ping'}. So, there probably is some struct I can ws.send that will be recognized as ping by the server, but the right way to get it is to run the proto script after adding the correct type to the chronik proto file. For now, subscribe is the only message I can send with existing and supported types.
But it's imo not worth adding the ping type to the chonik proto and running the script again, since the ultimate plan here is to have chronik itself send the pings, which is the proper way to do it. The only reason I think it's still worth landing this approach is because implementing the updated chronik (with auto backups) into Cashtab is a bit of a refactor, which I would like to do now, since it opens up work on other websocket improvements in Cashtab.
The change from client pings to server pings is not expected to require significant Cashtab refactor. Since we have already published chronik-client with client pings, and it is currently just not working for the browser, imo worth getting it fixed.