Repository navigation
fix(sdl): report a memory or storage size without a unit as invalid input - #4033
Conversation
…nput chain-sdk only checks a size string while it builds the manifest, and a value it cannot parse (for example "1073741824" with no unit) leaves generateManifest as a thrown Error rather than a validation result. The API had no handling for it, so the request answered 500. Convert that error into the same validation result every other SDL problem takes, naming the value and the expected format, so the request answers 400.
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 37 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 52 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Repository: akash-network/console/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (5)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4033 +/- ##
==========================================
- Coverage 87.74% 87.59% -0.16%
==========================================
Files 1281 1177 -104
Lines 36066 33394 -2672
Branches 8576 8048 -528
==========================================
- Hits 31647 29250 -2397
+ Misses 3911 3654 -257
+ Partials 508 490 -18
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward, well-tested bug fix.
What was reviewed: the new generateManifestReportingInvalidSizes wrapper (narrow catch on the Invalid size string: prefix, rethrows anything else); its wiring into SdlService.generateManifestFrom; and the new instancePath/message match the existing /profiles/compute and Invalid SDL: ${message} conventions already used elsewhere in sdl.service.ts and deployment-writer.service.ts. Coverage spans unit (sdl-sizes.spec.ts), service (sdl.service.spec.ts), and functional (deployments.spec.ts) levels, and the new tests follow the repo's setup() convention already used in the file.
Extended reasoning...
Small, self-contained fix (111 insertions, 2 deletions) adding a try/catch wrapper around chain-sdk's generateManifest to convert a raw thrown Error into a structured ValidationError; no auth, crypto, or data-exposure surface is touched. No prior findings and no third-party review comments to address. Verified the new instancePath and "Invalid SDL: ..." message format match existing patterns elsewhere in sdl.service.ts and deployment-writer.service.ts, and that tests follow the repo's setup() convention.
|
Matches the change: the catch is limited to the |
|
Upstream fix opened: akash-network/chain-sdk#362 makes the TS SDL parser accept a bare byte count the way the Go parser already does, with a parity fixture. Once that ships in a chain-sdk release and Console bumps, this 400 path becomes unreachable for bare numbers and can be dropped. |
1 participant
Why
POST /v1/deploymentsanswered 500 for an SDL whose memory or storagesizehad no unit (for example"1073741824"). chain-sdk validates sizes only while building the manifest and throws a plainErrorfor one it cannot parse, and the API's SDL pipeline only turned YAML and schema failures into 400s, so this escaped as an unhandled error and paged the 5xx alert. Twelve such requests hit prod on 2026-09-24.What
generateManifestReportingInvalidSizeswraps chain-sdk'sgenerateManifestand converts anInvalid size stringerror into the validation result every other SDL problem takes, naming the value and the expected format. Any other error still propagates.SdlService.generateManifestFromgoes through it, so both the unresolved and the resolved manifest paths answer 400 withInvalid SDL: memory or storage size "1073741824" must be a number with a unit, such as 512Mi or 1Gi.