stacks-archive / stacks-transactions-js

The JavaScript library for generating Stacks 2.0 transactions
19 stars 17 forks source link

fix: return response as json #104

Closed kyranjamie closed 4 years ago

kyranjamie commented 4 years ago

Ran into the slightly confusing functionality that the broadcastTransaction function returns the txid prequoted.

i.e. "f2754bbc5d6b041d220c759b5b9b05dc0b09337959ebb725fb1ab12ac9928c3d" not f2754bbc5d6b041d220c759b5b9b05dc0b09337959ebb725fb1ab12ac9928c3d

Any reason we specifically need .text() call here? I could JSON.parse it, of course, though unless there's a reason not to, think it's nicer to return the id directly.

Also curious if there's any reason not to have the hex encoded version with the leading 0x? Txids returned from sidecar have this prefix and I need to compare them.

Edit: need to use .clone() per this issue https://github.com/node-fetch/node-fetch/issues/533

yknl commented 4 years ago

Stacks nodes return TXID without the 0x prefix. We should probably standardize it one way or another.

As for parsing the JSON returned from the transaction broadcast, we should add error handling in case the response isn't JSON. Otherwise the error message gets replaced by a generic JSON parsing error.

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 4 years ago

Codecov Report

Merging #104 into master will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   84.10%   84.12%   +0.01%     
==========================================
  Files          28       28              
  Lines        1812     1814       +2     
  Branches      367      367              
==========================================
+ Hits         1524     1526       +2     
  Misses        286      286              
  Partials        2        2              
Impacted Files Coverage Δ
src/builders.ts 76.36% <100.00%> (+0.21%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9981f63...0ed6519. Read the comment docs.