Conversation
ogazboiz
left a comment
There was a problem hiding this comment.
The RPC timeout implementation is a good fix. Two things before this can merge:
1. Remove .kiro/settings/lsp.json
This is an IDE settings file from the Kiro editor and should not be in the repo:
git rm -r .kiro/
echo '.kiro/' >> .gitignore
git commit -m 'chore: remove Kiro IDE settings, add to gitignore'2. CI needs approval
Push the fix above and a maintainer will approve the CI run.
|
The codebase issues on main have been resolved and all CI checks are passing now. Please rebase your branch to pull in the latest changes before continuing. Thanks for your patience. |
|
@ogazboiz Pls review Pr |
ogazboiz
left a comment
There was a problem hiding this comment.
Committed a .kiro/settings/lsp.json editor config file, remove it. Massive package-lock.json noise from npm version mismatch. The timeout tests don't actually test the real fetchWithTimeout function. The rpcCall wrapper on the last line has an unclosed parenthesis. Fix and trigger CI.
- Add fetchWithTimeout in stellar.ts and inject it into rpc.Server so every SDK HTTP call (getAccount, prepareTransaction, sendTransaction, getHealth, pollTransaction) is subject to a hard 15-second abort. - Wrap all server.* calls in sorobanService.ts with rpcCall() which converts AbortError into a clean AppError.internal timeout message. - Add two tests to stellarConfig.test.ts covering signal injection and abort-on-timeout behaviour.
8641266 to
430c126
Compare
|
@ogazboiz pls review |
|
All four feedback items addressed, nice work. The diff is clean now. One thing: CI hasn't run on this branch. Push an empty commit to trigger it: git commit --allow-empty -m 'trigger CI' && git push |
|
@ogazboiz review |
|
heads up, a few important changes just landed on main that affect your PR:
please rebase on latest main: git fetch upstream
git rebase upstream/main
git push --force-with-lease |
|
@ogazboiz rebase done |
ogazboiz
left a comment
There was a problem hiding this comment.
hey @Sundriveauto, the RPC timeout with AbortController is a solid fix. the fetchWithTimeout wrapper and tests look good. merging!
if you want to keep contributing or follow up on open issues, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0
|
hey @Sundriveauto, approved and ready to merge but there's a small conflict now (likely .gitignore overlap with #601 which just merged). quick rebase should fix it: git fetch upstream
git rebase upstream/main
git push --force-with-leasewill merge as soon as it's clean. |
Closed #473
Backend Stellar RPC calls in sorobanService.ts use the SDK's default 30-second transaction timeout but do not configure socket-level connection timeouts. If the RPC node is slow or hangs after accepting the connection, the request never times out and the caller waits indefinitely. This can exhaust the Node.js event loop and cause all subsequent requests to queue up.
@ogazboiz pls review Pr