Replace request and xhr with axios #230

mainnet-pat · 2024-08-19T23:09:23Z

Resolves #95

woodser · 2024-08-19T23:12:17Z

Thanks. Can you remove the file changes to ./dist please?

mainnet-pat · 2024-08-19T23:28:50Z

I thought the wasm must be recompiled for this to work.

Or you want to recompile yourself?

Dist .js files contain old logic too

woodser · 2024-08-19T23:38:47Z

Yeah I always update the dist files at the end of the release on the master branch, since they contain large binaries and other generated artifacts which don't need to be in the repository, and which can cause merge conflicts on rebasing/testing with other commits. It also makes it easier to view the diffs.

mainnet-pat · 2024-08-20T00:03:41Z

Removed. Also tested the code in node and in firefox and brave. Because of the digest ping pong the console looks really ugly

mainnet-pat · 2024-08-20T07:46:38Z

Fixed handling of binary requests and responses, thoroughly tested on both daemon and wallet-rpc being auth-protected.

woodser · 2024-08-20T13:04:06Z

src/test/Axios.ts

+import { createWalletFull, MoneroNetworkType, MoneroWalletListener } from "../..";
+import { connectToWalletRpc } from "../..";
+
+describe("Axios", function() {


Thanks, but let's actually remove this test, since it's redundant with existing tests like the sample code.

woodser · 2024-08-20T13:05:07Z

src/test/TestAll.ts

@@ -57,3 +57,5 @@ new TestMoneroConnectionManager().runTests();

 // test scratchpad
 require("./Scratchpad");
+
+require("./Axios");


Can remove this too.

woodser · 2024-08-20T14:30:45Z

I'm getting an error building the dist files myself, specifically the web worker:

[100%] Built target monero_wallet_full

> monero-ts@0.10.0 build_web_worker
> webpack --config ./webpack.worker.js

assets by status 1.78 MiB [cached] 97 assets
runtime modules 1010 bytes 5 modules
modules by path ./node_modules/ 2.01 MiB
  javascript modules 2 MiB 274 modules
  json modules 12.7 KiB
    modules by path ./node_modules/browserify-sign/ 2.23 KiB 2 modules
    + 4 modules
modules by path ./src/main/ts/ 835 KiB
  modules by path ./src/main/ts/wallet/ 438 KiB 33 modules
  modules by path ./src/main/ts/daemon/ 211 KiB 30 modules
  modules by path ./src/main/ts/common/*.ts 186 KiB 14 modules
modules by path ./dist/*.js 460 KiB
  ./dist/monero_wallet_keys.js 110 KiB [built] [code generated]
  ./dist/monero_wallet_full.js 350 KiB [built] [code generated]
+ 15 modules

ERROR in /Users/woodser/git/monero-ts/index.ts
./index.ts 1:0-12
[tsl] ERROR in /Users/woodser/git/monero-ts/index.ts(1,1)
      TS6200: Definitions of the following identifiers conflict with those in another file: GenUtils, Filter, MoneroError, HttpClient, LibraryUtils, MoneroRpcConnection, MoneroRpcError, SslOptions, TaskLooper, ConnectionType, MoneroAltChain, MoneroBan, MoneroBlockHeader, MoneroBlock, MoneroBlockTemplate, MoneroConnectionSpan, MoneroDaemonConfig, MoneroDaemonInfo, MoneroDaemonListener, MoneroDaemonSyncInfo, MoneroDaemonUpdateCheckResult, MoneroDaemonUpdateDownloadResult, MoneroFeeEstimate, MoneroHardForkInfo, MoneroKeyImage, MoneroKeyImageSpentStatus, MoneroMinerTxSum, MoneroMiningStatus, MoneroNetworkType, MoneroOutput, MoneroOutputHistogramEntry, MoneroSubmitTxResult, MoneroTx, MoneroTxPoolStats, MoneroVersion, MoneroPeer, MoneroPruneResult, MoneroAccount, MoneroAccountTag, MoneroAddressBookEntry, MoneroCheck, MoneroCheckReserve, MoneroCheckTx, MoneroDestination, MoneroIntegratedAddress, MoneroKeyImageImportResult, MoneroMultisigInfo, MoneroMultisigInitResult, MoneroMultisigSignResult, MoneroOutputWallet, MoneroOutputQuery, MoneroTxPriority, MoneroTxConfig, MoneroSubaddress, MoneroSyncResult, MoneroTransfer, MoneroIncomingTransfer, MoneroOutgoingTransfer, MoneroTransferQuery, MoneroTxSet, MoneroTxWallet, MoneroTxQuery, MoneroWalletListener, MoneroWalletConfig, MoneroMessageSignatureType, MoneroMessageSignatureResult, MoneroConnectionManagerListener, MoneroConnectionManager, MoneroDaemon, MoneroWallet, MoneroDaemonRpc, MoneroWalletRpc, MoneroWalletKeys, MoneroWalletFull, MoneroUtils, ThreadPool
ts-loader-default_e3b0c44298fc1c14
 @ ./dist/monero_wallet_full.js 9:10941-10960 9:12280-12299
 @ ./src/main/ts/common/LibraryUtils.ts 97:27-73
 @ ./src/main/ts/common/MoneroWebWorker.ts 9:39-64

Monero web worker config (webpack 5.88.2) compiled with 1 error in 7172 ms

This is after deleting ./build, ./browser_build, and ./node_modules, running npm install, then ./bin/build_all.sh for a full rebuild.

mainnet-pat · 2024-08-20T14:40:29Z

Yeah I had this error. I had to do:

rm -rf dist/
./bin/build_all.sh

Maybe it is the quirk of new emscription? It seemingly have nothing to do with my changes

woodser · 2024-08-20T14:45:09Z

That did the trick :)

woodser · 2024-08-20T15:29:17Z

Hm, it's strange though. I get this error every time running ./bin/build_dist.sh without first deleting the dist directory. But this behavior only happens with this PR applied. Otherwise it always works.

We're going to have to figure this out before I can incorporate this change, unfortunately.

mainnet-pat · 2024-08-20T16:22:24Z

Can confirm this on master, after removing new redundant axios tests the issue went away. Please try again.

I have omitted pushing dist changes as you asked

woodser · 2024-08-20T16:27:46Z

That works :)

mainnet-pat · 2024-08-20T16:32:10Z

perfect

woodser · 2024-08-21T00:05:55Z

The tests are all working, except one failure on "Can manage connections", which passes on master.

In case it's related, this part of the code is making requests to multiple monero-wallet-rpc servers simultaneously, in order to test getting the best connection.

The easiest way to run this test is in Node.js, with npm run test -- --grep "Can manage connections", after setting MONERO_BINS_DIR in TestUtils.ts to the directory of your monerod and monero-wallet-rpc binaries.

If you want to run the test in the browser, run ./bin/start_wallet_rpc_test_servers.sh in another terminal to pre-start monero-wallet-rpc processes for the test to use, then ./bin/build_dist.sh && ./bin/build_browser.sh, then run the test at this link: http://localhost:8080/tests.html?grep=Test%20connection%20manager%20Can%20manage%20connections

mainnet-pat · 2024-08-21T11:55:07Z

Fixed the failing test.

My observation: await connectionManager.checkConnections(); invokes await connection.checkConnection() which sends get_blocks_by_height.bin binary request to wallet rpc, not a daemon, so it is always 404.

But if it is indeed daemon it asks for first 100 blocks which might be a bit too heavy for a connection check. Maybe something like get_block_count or get_version would suffice?

woodser · 2024-08-21T12:50:22Z

My observation: await connectionManager.checkConnections(); invokes await connection.checkConnection() which sends get_blocks_by_height.bin binary request to wallet rpc, not a daemon, so it is always 404.

Yeah it assumes a connection to monerod (almost always true), and falls back to treating a 404 as connected and uses the latency check.

But if it is indeed daemon it asks for first 100 blocks which might be a bit too heavy for a connection check. Maybe something like get_block_count or get_version would suffice?

The idea is to get a measurement which reflects if the node is under load, with the first 100 blocks being relatively light compared to later blocks for fully syncing the wallet.

woodser

Everything works. Just some final cleanup requested.

woodser · 2024-08-21T15:25:38Z

src/main/ts/common/HttpClient.ts

+
+    // normalize response
+    let normalizedResponse: any = {};
+


Remove empty line

woodser · 2024-08-21T15:25:44Z

src/main/ts/common/HttpClient.ts

+
+    normalizedResponse.statusCode = resp.status;
+    normalizedResponse.statusText = resp.statusText;
+


Remove empty line

woodser · 2024-08-21T15:26:05Z

src/main/ts/common/HttpClient.ts

@@ -200,7 +119,7 @@ export default class HttpClient {
    });
    return HttpClient.HTTPS_AGENT;
  }
-  
+
  protected static parseXhrResponseHeaders(headersStr) {


This function can be removed completely now.

woodser · 2024-08-21T15:26:27Z

src/main/ts/common/HttpClient.ts

+        const randNum = Math.round(Math.random() * characters.length);
+        token += characters.slice(randNum, randNum+1);
+      }
+      return token;
    }



Remove extra empty line

mainnet-pat · 2024-08-21T16:13:10Z

Done

mainnet-pat force-pushed the replace_request_with_axios branch from a083e9f to 735644c Compare August 19, 2024 23:31

mainnet-pat force-pushed the replace_request_with_axios branch from 735644c to f8cb5c6 Compare August 20, 2024 00:01

mainnet-pat force-pushed the replace_request_with_axios branch from f8cb5c6 to 3ead35f Compare August 20, 2024 07:45

woodser reviewed Aug 20, 2024

View reviewed changes

Replace request and xhr with axios

69fbe96

mainnet-pat force-pushed the replace_request_with_axios branch from 3ead35f to 69fbe96 Compare August 20, 2024 16:21

Fix axios error handling

909cb9d

woodser reviewed Aug 21, 2024

View reviewed changes

Cleanup

f0344a8

woodser merged commit 2e78584 into woodser:master Aug 21, 2024

This was referenced Aug 22, 2024

Polyfill with axios woodser/xmr-sample-vite#3

Closed

Fix vite sample application [2 XMR] #229

Closed

vite browser example #202

Closed

woodser mentioned this pull request Sep 18, 2024

[feature] Make it easier to use monero-javascript for front end development #83

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace request and xhr with axios #230

Replace request and xhr with axios #230

mainnet-pat commented Aug 19, 2024 •

edited

Loading

woodser commented Aug 19, 2024 •

edited

Loading

mainnet-pat commented Aug 19, 2024 •

edited

Loading

woodser commented Aug 19, 2024

mainnet-pat commented Aug 20, 2024 •

edited

Loading

mainnet-pat commented Aug 20, 2024

woodser Aug 20, 2024 •

edited

Loading

mainnet-pat Aug 20, 2024

woodser Aug 20, 2024

mainnet-pat Aug 20, 2024

woodser commented Aug 20, 2024

mainnet-pat commented Aug 20, 2024 •

edited

Loading

woodser commented Aug 20, 2024

woodser commented Aug 20, 2024

mainnet-pat commented Aug 20, 2024

woodser commented Aug 20, 2024

mainnet-pat commented Aug 20, 2024

woodser commented Aug 21, 2024 •

edited

Loading

mainnet-pat commented Aug 21, 2024 •

edited

Loading

woodser commented Aug 21, 2024

woodser left a comment

woodser Aug 21, 2024

woodser Aug 21, 2024

woodser Aug 21, 2024

woodser Aug 21, 2024

mainnet-pat commented Aug 21, 2024


		normalizedResponse.statusCode = resp.status;
		normalizedResponse.statusText = resp.statusText;

Replace request and xhr with axios #230

Replace request and xhr with axios #230

Conversation

mainnet-pat commented Aug 19, 2024 • edited Loading

woodser commented Aug 19, 2024 • edited Loading

mainnet-pat commented Aug 19, 2024 • edited Loading

woodser commented Aug 19, 2024

mainnet-pat commented Aug 20, 2024 • edited Loading

mainnet-pat commented Aug 20, 2024

woodser Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

mainnet-pat Aug 20, 2024

Choose a reason for hiding this comment

woodser Aug 20, 2024

Choose a reason for hiding this comment

mainnet-pat Aug 20, 2024

Choose a reason for hiding this comment

woodser commented Aug 20, 2024

mainnet-pat commented Aug 20, 2024 • edited Loading

woodser commented Aug 20, 2024

woodser commented Aug 20, 2024

mainnet-pat commented Aug 20, 2024

woodser commented Aug 20, 2024

mainnet-pat commented Aug 20, 2024

woodser commented Aug 21, 2024 • edited Loading

mainnet-pat commented Aug 21, 2024 • edited Loading

woodser commented Aug 21, 2024

woodser left a comment

Choose a reason for hiding this comment

woodser Aug 21, 2024

Choose a reason for hiding this comment

woodser Aug 21, 2024

Choose a reason for hiding this comment

woodser Aug 21, 2024

Choose a reason for hiding this comment

woodser Aug 21, 2024

Choose a reason for hiding this comment

mainnet-pat commented Aug 21, 2024

mainnet-pat commented Aug 19, 2024 •

edited

Loading

woodser commented Aug 19, 2024 •

edited

Loading

mainnet-pat commented Aug 19, 2024 •

edited

Loading

mainnet-pat commented Aug 20, 2024 •

edited

Loading

woodser Aug 20, 2024 •

edited

Loading

mainnet-pat commented Aug 20, 2024 •

edited

Loading

woodser commented Aug 21, 2024 •

edited

Loading

mainnet-pat commented Aug 21, 2024 •

edited

Loading