Revert #3331: don't throw on missing network_id in getServerInfo

[ckeshava]

Reverts the code changes from #3331.

xrpl.js must not enforce network_id rules more strictly than a rippled node does for custom XRPL networks. Client.getServerInfo() again logs server_info failures via console.error and leaves client.networkID undefined instead of throwing, and the test added in #3331 is removed.

HISTORY.md keeps the original 5.0.0 changelog lines and instead gains a new Unreleased → Fixed entry recording the revert.

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Client.getServerInfo() is changed to wrap its server_info request and networkID/buildVersion assignment in a try/catch, logging failures via console.error and swallowing the exception instead of throwing. The JSDoc is trimmed to match. The entire getServerInfo test suite (167 lines) is deleted, and a changelog entry documents the revert.

Changes

Revert getServerInfo throwing behavior

Layer / File(s)	Summary
getServerInfo try/catch, JSDoc, and changelog packages/xrpl/src/client/index.ts, packages/xrpl/HISTORY.md, packages/xrpl/test/client/getServerInfo.test.ts	getServerInfo() now wraps the server_info request and networkID/buildVersion assignment in a try/catch, calling console.error on failure instead of throwing; the JSDoc removes the throws contract; the changelog documents the revert; and the getServerInfo test suite is entirely deleted.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• XRPLF/xrpl.js#3331: This PR directly reverts the behavioral change introduced there, switching Client.getServerInfo() back from throwing/strict networkID validation to console.error logging with an undefined networkID, and removes the corresponding tests.

Suggested reviewers

• Patel-Raj11
• kuan121

Poem

🐇 Oh the network_id was lost in the mist,
A throw in the code that shouldn't exist!
Now a soft console.error takes its place,
And networkID undefined is no disgrace.
Hop along, client — connect with no fear,
The rabbit reverts and the error logs clear! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	It explains the revert and HISTORY update, but it omits the template sections for type of change, HISTORY checkbox, and test plan.	Add the required template sections with change type checkboxes, a clear HISTORY.md answer, and a reproducible test plan.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main revert and the missing network_id behavior change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: one or more packages not found in the registry.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/xrpl/src/client/index.ts (1)

539-547: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep a regression test for the restored behavior.

Since connect() depends on this method resolving, add a focused test that server_info rejection logs via console.error, connect()/getServerInfo() resolves, and networkID remains undefined for a fresh client.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/xrpl/src/client/index.ts` around lines 539 - 547, The error handling
in the server_info fetch path should preserve the restored connect behavior
while being covered by regression tests. Update the tests around the
getServerInfo()/connect() flow in Client to verify that a rejected server_info
request is logged through console.error, that connect() still resolves
successfully, and that a fresh client leaves networkID undefined after the
failure. Use the Client methods getServerInfo and connect, and assert the
console.error side effect plus the unresolved info state for a new client
instance.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/xrpl/src/client/index.ts`:
- Around line 526-528: Update the JSDoc for getServerInfo to document its
non-throwing failure behavior: when server_info fails, the promise still
resolves, the existing fields may remain stale, and the error is logged. Make
sure the comment near getServerInfo and any related return description reflect
this contract clearly so callers do not expect an exception.

---

Nitpick comments:
In `@packages/xrpl/src/client/index.ts`:
- Around line 539-547: The error handling in the server_info fetch path should
preserve the restored connect behavior while being covered by regression tests.
Update the tests around the getServerInfo()/connect() flow in Client to verify
that a rejected server_info request is logged through console.error, that
connect() still resolves successfully, and that a fresh client leaves networkID
undefined after the failure. Use the Client methods getServerInfo and connect,
and assert the console.error side effect plus the unresolved info state for a
new client instance.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 362ed722-3cd1-4d1a-862d-31f7e6675f9b

📥 Commits

Reviewing files that changed from the base of the PR and between 7791ce2 and b6d1922.

📒 Files selected for processing (3)

• packages/xrpl/HISTORY.md
• packages/xrpl/src/client/index.ts
• packages/xrpl/test/client/getServerInfo.test.ts

💤 Files with no reviewable changes (1)

• packages/xrpl/test/client/getServerInfo.test.ts