[chronik-client] Do not treat node reject tx error as chronik server connection error
AbandonedPublic
Actions

Authored by bytesofman on Jan 12 2024, 17:14.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Summary

If the node rejects a tx for any reason, chronik returns the error with bitcoind-rejected-tx in the error msg. Currently this is treated by chronik as a server issue, causing failoverProxy to cycle through all available nodes -- which will all throw the same error -- which will then return an unuseful 'chronik unavailable' msg to the user.

Do not move to the next chronik instance if you have a node rejection error and instead return that error.

Test Plan

cd modules/chronik-client && npm run build
In Cashtab, npm i ../modules/chronik-client on this branch
npm start
Send txs, then open same wallet in another device, send a tx before utxo set has refreshed
Confirm you get the bitcoind-rejected-tx error instead chronik unavailable error

Diff Detail

Repository

rABC Bitcoin ABC

Branch

chronik-client-patch

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26295
Build 52160: Build Diff	chronik-client-integration-tests · chronik-client-tests
Build 52159: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jan 12 2024, 17:14

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 12 2024, 17:14

before

after

Harbormaster completed remote builds in B26295: Diff 44102.Jan 12 2024, 17:21

Fabien added a subscriber: Fabien.Jan 12 2024, 18:47

Fabien added inline comments.

modules/chronik-client/src/failoverProxy.ts
130	Any chance we can subtype the chronik errors to filter and not having to look at the message at all which is very brittle ?
133	how is this even possible ? what does an error message look like that has both bitcoind-rejected-tx and one of the other parts as well ?

bytesofman added inline comments.Jan 12 2024, 19:22

modules/chronik-client/src/failoverProxy.ts
130	indeed the solution here presented is not the best way to fix it, though it will fix the current prod ux. It is not particularly urgent provided I get the "update utxo set on load" diff landed, which is the main reason this error is coming up in prod.
133	Yes, chronik should probably handle this differently. At the moment it happens with `bad utxos missing or spent` Chronik will return `Failed getting /broadcast-tx` along with the utxo msg, prefaced by `bitcoind-rejected-tx`

Fabien requested changes to this revision.Jan 15 2024, 11:27

Fabien added inline comments.

modules/chronik-client/src/failoverProxy.ts
130	OK let's do the right thing then , because it is guaranteed to break in no time by relying on the special case error string

This revision now requires changes to proceed.Jan 15 2024, 11:27

Review error messages generated in chronik
Is there an error code that distinguishes between bad connection error and node rejection error?
If not, need to implement this in chronik
Use this code to make chronik-client more robust against API failures not related to connection issues