Handle Error with basic retry while syncing external assets ( Price Data ) #1691
Conversation
| "MEMPOOL_API": "https://mempool.space/api/v1", | ||
| "MEMPOOL_ONION": "http://mempoolhqx4isw62xs7abwphsq7ldayuidyx2v2oethdhhj6mlo2r6ad.onion/api/v1", | ||
| "LIQUID_API": "https://liquid.network/api/v1", | ||
| "LIQUID_ONION": "http://liquidmom47f6s3m53ebfxn47p76a6tlnxib3wp6deux7wuzotdr6cyd.onion/api/v1" | ||
| } |
There was a problem hiding this comment.
Exposed static URLs used to fetch resources to mempool-config.json ( So it could be updated by users if they have an alternative & Would be better to manage external resources )
| const data = await axios.get(path, axiosOptions); | ||
| if (data.statusText !== 'OK' || !data.data) { | ||
| const data: AxiosResponse = await axios.get(path, axiosOptions); | ||
| if (data.statusText === 'error' || !data.data) { |
There was a problem hiding this comment.
Changed this one since not every servers respond with statusText === "OK"
{
status: 200,
statusText: '',
headers: {
'content-type': 'application/json',
'transfer-encoding': 'chunked',
date: 'Fri, 20 May 2022 13:54:11 GMT',
connection: 'close'
},
config: {
transitional: {
silentJSONParsing: true,
forcedJSONParsing: true,
clarifyTimeoutError: false
},
adapter: [Function: httpAdapter],
transformRequest: [ [Function: transformRequest] ],
transformResponse: [ [Function: transformResponse] ],
timeout: 30000,
xsrfCookieName: 'XSRF-TOKEN',
xsrfHeaderName: 'X-XSRF-TOKEN',
maxContentLength: -1,
maxBodyLength: -1,
env: { FormData: [Function] },
validateStatus: [Function: validateStatus],
headers: {
Accept: 'application/json, text/plain, */*',
'User-Agent': 'mempool/v2.4.0-dev'
},
httpAgent: SocksProxyAgent {
_events: [Object: null prototype] {},
_eventsCount: 0,
_maxListeners: undefined,
timeout: null,
maxFreeSockets: 1,
maxSockets: 1,
maxTotalSockets: Infinity,
sockets: {},
freeSockets: {},
requests: {},
options: {},
shouldLookup: false,
proxy: [Object],
tlsConnectionOptions: {},
promisifiedCallback: [Function: callback],
[Symbol(kCapture)]: false
},
method: 'get',
url: 'http://wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid.onion/getAllMarketPrices',
data: undefined
},
} | throw new Error(`Could not fetch data from Github, Error: ${data.status}`); | ||
| } | ||
| return data.data; | ||
| } catch (e) { | ||
| logger.err('Could not connect to Github. Reason: ' + (e instanceof Error ? e.message : e)); | ||
| retry++; | ||
| } | ||
| await setDelay(); | ||
| await setDelay(config.MEMPOOL.EXTERNAL_RETRY_INTERVAL); |
There was a problem hiding this comment.
Exposed this value to configuration as per discussed from #1649 (comment)
|
This PR is an attempt to add missing configuration value & handle errors with retrys for unstable networks like tor. |
There was a problem hiding this comment.
@ayanamidev thanks for these changes.
When adding new variables to the config file, we also need to update a few other things related to the Docker image:
- The README sections that explains the overrides:
Line 73 in b4beb29
- The template backend config file that is used by the startup script:
- The startup script itself, that replaces the values in the template with defaults or the overrides specified in the docker-compose.yml file:
mempool/docker/backend/start.sh
Line 74 in b4beb29
- The docker-compose-yml file depending on what's being added. In your case you don't need to add any overrides at this time.
backend/src/config.ts
Outdated
| 'MEMPOOL_API': 'https://mempool.space/api/v1', | ||
| 'MEMPOOL_ONION': 'http://mempoolhqx4isw62xs7abwphsq7ldayuidyx2v2oethdhhj6mlo2r6ad.onion/api/v1', | ||
| 'LIQUID_API': 'https://liquid.network/api/v1', | ||
| 'LIQUID_ONION': 'http://liquidmom47f6s3m53ebfxn47p76a6tlnxib3wp6deux7wuzotdr6cyd.onion/api/v1' |
There was a problem hiding this comment.
@ayanamidev I think these should be called MEMPOOL_CLEARNET, LIQUID_CLEARNET and BISQ_CLEARNET, containing just the base URL, then /api/v1 gets appended at the time you make the request.
There was a problem hiding this comment.
@knorrium In case that the configuration should be changed to connect localhost instance without the help of the nginx reverse proxy.
https://mempool.space/api/donations
Also, accessing /api/donations endpoint will result in endpoint does not exist "/donations" error.
https://mempool.space/api/v1/donations
While this one works well
| response = await axios.get(fiatConversionUrl, { httpAgent: agent, headers: headers, timeout: 30000 }); | ||
| } else { | ||
| fiatConversionUrl = config.PRICE_DATA_SERVER.CLEARNET_URL; | ||
| while(retry < config.MEMPOOL.EXTERNAL_MAX_RETRY) { |
There was a problem hiding this comment.
@ayanamidev I missed this on the previous PR but the retries are blocking the backend from performing other tasks. Notice the logs after getting 403s below
May 23 21:37:58 [99503] NOTICE: Mempool Server is running on port 8999
May 23 21:37:58 [99503] INFO: Updating latest mining pools from Github
May 23 21:37:58 [99503] INFO: List of public pools will be queried over the Tor network
May 23 21:37:58 [99503] ERR: Cannot fetch pools.json sha from db. Reason: connect ENOENT /var/run/mysql/mysql.sock
May 23 21:37:59 [99503] ERR: Could not connect to Github. Reason: Request failed with status code 403
May 23 21:37:59 [99503] DEBUG: USD Conversion Rate: 29306.538
May 23 21:39:00 [99503] ERR: Could not connect to Github. Reason: Request failed with status code 403
May 23 21:40:01 [99503] ERR: Could not connect to Github. Reason: Request failed with status code 403
May 23 21:41:03 [99503] ERR: Could not connect to Github. Reason: Request failed with status code 403
May 23 21:42:04 [99503] ERR: Could not connect to Github. Reason: Request failed with status code 403
May 23 21:43:05 [99503] DEBUG: Pools.json sha | Current: undefined | Github: cc8d3e01dea2787525ed1f182a0f9a685fe3a850
May 23 21:43:05 [99503] WARN: Pools.json is outdated, fetch latest from github
May 23 21:43:07 [99503] DEBUG: Parse coinbase_tags
May 23 21:43:07 [99503] DEBUG: Parse payout_addresses
May 23 21:43:07 [99503] DEBUG: Identify unique mining pools
May 23 21:43:07 [99503] DEBUG: Found 136 unique mining pools
May 23 21:43:07 [99503] ERR: Cannot get existing pools from the database, skipping pools.json import
May 23 21:43:07 [99503] ERR: Cannot save github pools.json sha into the db. Reason: connect ENOENT /var/run/mysql/mysql.sock
May 23 21:43:07 [99503] NOTICE: PoolsUpdater completed
May 23 21:43:07 [99503] DEBUG: Initial difficulty adjustment data set.
May 23 21:43:07 [99503] DEBUG: New block found (#737663)!
May 23 21:43:07 [99503] DEBUG: Indexing tx 1 of 830 in block #737663
May 23 21:43:07 [99503] DEBUG: 0 of 830 found in mempool. 1 fetched through backend service.
or when we can't connect to the proxy:
May 23 22:00:04 [102230] INFO: Updating latest mining pools from Github
May 23 22:00:04 [102230] INFO: List of public pools will be queried over the Tor network
May 23 22:00:04 [102230] ERR: Cannot fetch pools.json sha from db. Reason: connect ENOENT /var/run/mysql/mysql.sock
May 23 22:00:04 [102230] ERR: Could not connect to Github. Reason: connect ECONNREFUSED 127.0.0.1:9051
retry: 1
May 23 22:01:04 [102230] DEBUG: Querying currency rates service...
May 23 22:01:04 [102230] ERR: Error updating fiat conversion rates: connect ECONNREFUSED 127.0.0.1:9051
May 23 22:01:04 [102230] ERR: Could not connect to Github. Reason: connect ECONNREFUSED 127.0.0.1:9051
retry: 2
May 23 22:02:04 [102230] DEBUG: Querying currency rates service...
May 23 22:02:04 [102230] ERR: Error updating fiat conversion rates: connect ECONNREFUSED 127.0.0.1:9051
May 23 22:02:04 [102230] ERR: Could not connect to Github. Reason: connect ECONNREFUSED 127.0.0.1:9051
retry: 3
There was a problem hiding this comment.
This seems odd, maybe the code isn't asynchronous and would run with await keyword instead which is not very efficient as well. Would be good to address this issue as well.
There was a problem hiding this comment.
@knorrium I believe you should ask @nymkappa about the issue since the reason for blocking tasks is mainly the pool updater has 'await' keyword to wait until Promise resolves https://github.com/mempool/mempool/blame/master/backend/src/index.ts#L155.
And since the indexer function https://github.com/mempool/mempool/blame/master/backend/src/index.ts#L158 depends on https://github.com/mempool/mempool/blame/master/backend/src/index.ts#L155, removing the await keyword would likely break other updater's behavior.
So my recommendation is to go as is but if people want, they would be able to reduce the retry limit per job.
There was a problem hiding this comment.
Have addressed issues with Tor connections a1fbe58, maybe try this commit.
backend/mempool-config.sample.json
Outdated
| ], | ||
| "EXTERNAL_ASSETS": [], | ||
| "EXTERNAL_MAX_RETRY": 10, | ||
| "EXTERNAL_RETRY_INTERVAL": 60, |
There was a problem hiding this comment.
@ayanamidev this should be in milliseconds to match POLL_RATE_MS set just a few lines above
There was a problem hiding this comment.
@knorrium Checking this again looks like there is a similar config called PRICE_FEED_UPDATE_INTERVAL which is a config defined with seconds not ms. If the config key doesn't include the term MS, wouldn't it be good to leave this value as is?
backend/package.json
Outdated
| "ng": "node ./node_modules/@angular/cli/bin/ng", | ||
| "tsc": "node ./node_modules/typescript/bin/tsc", |
There was a problem hiding this comment.
This looks committed by mistake - obviously not related to price data :)
There was a problem hiding this comment.
It isn't, maybe I should separate the PR to avoid confusion, but this is what enables building for Windows environment
|
Sorry, what is the goal of this PR exactly? The linked issue doesn't seem to be related to the majority of the PR, so I'm very confused. |
|
@wiz Adding |
|
What is the goal of this PR exactly? |
Most of the features of this PR is focused to enhance privacy & avoid immediate failure for weak internet connection or rate limits. |
|
Rebased conflicts |
|
@ayanamidev could you please rebase onto current master and also it would be nice if you can enable the "allow maintainers to make edits" option |
…ata ) + Removed unused External Assets value + Make static URL dynamic + Added config options for syncing pool data + Added retry interval & max retry
Will use `mempool/v${backendInfo.getBackendInfo().version}` for default
addressing comments from @knorrium
To address error Socks5 proxy rejected connection - Failure
Until we find out how to sync async
|
@wiz Rebase done About the option, I could not find one from https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork. Could you check the repo settings? |
|
@softsimon Shouldn't the TSLint considered as deprecated? https://www.npmjs.com/package/tslint I use ESLint for TypeScript btw https://github.com/ayanamitech/axios-auto/blob/main/.eslintrc.json |
User-Agentheader value