fix: trim and validate network string input#92
fix: trim and validate network string input#92owizdom wants to merge 1 commit intokeep-starknet-strange:mainfrom
Conversation
Network strings with trailing whitespace (e.g. "sepolia\n" from env vars) silently fail with a misleading "network not specified" error. Now trims input and throws a descriptive error listing valid options. Closes keep-starknet-strange#89 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded runtime validation for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/sdk.ts`:
- Around line 96-106: The error message currently replaces whitespace using
config.network.replace(/\s/g, "\\n") which misrepresents spaces and other
control chars; update the throw in the network parsing block (the branch
handling typeof config.network === "string" that sets networkPreset) to escape
the provided network string using JSON.stringify(config.network.trim()) (or
equivalent) so spaces, tabs, newlines and other control characters are shown
accurately in the Error text alongside the valid options list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7cbb8a25-f854-45da-aef0-469d65c21475
📒 Files selected for processing (1)
src/sdk.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use@/path alias for internal imports in TypeScript source files
Use strict TypeScript compiler settings:strict,noUncheckedIndexedAccess,exactOptionalPropertyTypesenabled
Files:
src/sdk.ts
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve strict TypeScript typing; prefer explicit domain types (Address, Amount, ChainId) over primitives
Files:
src/sdk.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not introduce CommonJS patterns; use ESM only
Files:
src/sdk.ts
{src/sdk.ts,src/wallet/**,src/types/**}
📄 CodeRabbit inference engine (AGENTS.md)
Must serialize: Concurrent edits in
src/sdk.ts,src/wallet/*, or sharedsrc/types/*
Files:
src/sdk.ts
src/**
⚙️ CodeRabbit configuration file
src/**: This is a public SDK. Prioritize:
- Backwards compatibility and clear, typed public APIs
- Deterministic behavior across mainnet/sepolia presets
- Tests for behavior changes (prefer unit tests)
Files:
src/sdk.ts
🧠 Learnings (2)
📚 Learning: 2026-03-06T10:04:13.920Z
Learnt from: CR
Repo: keep-starknet-strange/starkzap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:04:13.920Z
Learning: Applies to src/index.ts : Keep public API changes intentional and export them via `src/index.ts`
Applied to files:
src/sdk.ts
📚 Learning: 2026-03-09T13:39:32.603Z
Learnt from: adrienlacombe
Repo: keep-starknet-strange/starkzap PR: 62
File: src/tx/builder.ts:449-539
Timestamp: 2026-03-09T13:39:32.603Z
Learning: Guideline for confidential transaction handling in StarkZap: In src/tx/builder.ts and src/confidential/types.ts, the sender field for confidential/tongo operations (ConfidentialFundDetails, ConfidentialTransferDetails, ConfidentialWithdrawDetails, ConfidentialRagequitDetails, ConfidentialRolloverDetails) represents the Starknet account submitting the transaction (often a relayer), not necessarily the wallet owner. The optional feeTo field is for relayed transactions where sender differs from the wallet.
Actionable checks for code reviews:
- Do not override details.sender with this.wallet.address in TxBuilder confidential methods; doing so breaks the relayer pattern.
- Ensure staking methods continue to bind this.wallet.address to reflect the pool member’s own address (no relayer concept there).
- If using a relayer, verify feeTo is set when appropriate and that sender remains the relayer address.
- Update or add tests to reflect the correct sender semantics for confidential operations...
Applied to files:
src/sdk.ts
🔇 Additional comments (1)
src/sdk.ts (1)
96-103: LGTM on the validation logic.The trimming and lookup logic correctly addresses the PR objective—strings like
"sepolia\n"from environment variables will now be trimmed before lookup, and invalid values produce a clear, actionable error with valid options listed.
| if (typeof config.network === "string") { | ||
| const trimmed = config.network.trim() as NetworkName; | ||
| networkPreset = networks[trimmed]; | ||
| if (!networkPreset) { | ||
| throw new Error( | ||
| `Invalid network "${config.network.replace(/\s/g, "\\n")}". Valid options: ${Object.keys(networks).join(", ")}` | ||
| ); | ||
| } | ||
| } else { | ||
| networkPreset = config.network; | ||
| } |
There was a problem hiding this comment.
Fix whitespace escaping in error message to avoid misrepresentation.
The regex /\s/g replaces all whitespace characters (spaces, tabs, carriage returns) with the literal string \n, which misrepresents the actual invalid input. A trailing space would be displayed as "sepolia\n" instead of indicating it was a space.
Consider using JSON.stringify() for accurate escaping of all control characters:
Proposed fix for accurate whitespace display
if (!networkPreset) {
throw new Error(
- `Invalid network "${config.network.replace(/\s/g, "\\n")}". Valid options: ${Object.keys(networks).join(", ")}`
+ `Invalid network ${JSON.stringify(config.network)}. Valid options: ${Object.keys(networks).join(", ")}`
);
}This produces proper escapes: "sepolia\n" for newlines, "sepolia\t" for tabs, "sepolia " for spaces, making debugging straightforward.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk.ts` around lines 96 - 106, The error message currently replaces
whitespace using config.network.replace(/\s/g, "\\n") which misrepresents spaces
and other control chars; update the throw in the network parsing block (the
branch handling typeof config.network === "string" that sets networkPreset) to
escape the provided network string using JSON.stringify(config.network.trim())
(or equivalent) so spaces, tabs, newlines and other control characters are shown
accurately in the Error text alongside the valid options list.
0xLucqs
left a comment
0xLucqs
left a comment
There was a problem hiding this comment.
coderabbit suggested something that makes sense imo
|
hello hello still interested ? |
2 participants
Problem
Network strings with trailing whitespace (e.g.
"sepolia\n"from environment variables) cause a misleading error:The network was specified — it just has a trailing newline.
networks["sepolia\n"]returnsundefined, and the error then claims network wasn't provided at all.This is common when env vars are copy-pasted from dashboards (Vercel, Railway) that sometimes include trailing whitespace.
Fix
.trim()the network string before lookupContext
Found while building Zapp (28 SDK modules used). Lost hours debugging this before spotting raw
\nbytes in a Vercel env var.Closes #89