fix: add timeout to client request in convertScratchToXbp function#3090
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a longer request timeout for the Scratch → XBuilder conversion API call so larger/longer conversions don’t fail due to the shared client’s default 10s timeout.
Changes:
- Introduce a
convertTimeoutconstant (60s) for Scratch conversion. - Pass
timeout: convertTimeoutto the/util/sb2xbpbinary POST request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 60-second timeout for the Scratch-to-XBuilder project conversion API call. The feedback suggests increasing this timeout to 5 minutes to ensure that larger projects (up to 32MB) have sufficient time for both upload and backend processing, especially on slower network connections.
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds a 60-second explicit timeout to the Scratch conversion request, which is clearly the right direction — the default 10-second client timeout is far too short for a server-side file conversion of up to 32 MB. A few things worth addressing:
| const convertTimeout = 60 * 1000 // 60s | ||
|
|
||
| /** | ||
| * Convert Scratch project (.sb2/.sb3) to XBuilder project (.xbp) via backend `/scratch-convert` endpoint. |
There was a problem hiding this comment.
The JSDoc (line 9 in the new file) says the endpoint is /scratch-convert but the actual call uses /util/sb2xbp — these don't match. Pre-existing, but worth fixing while touching this file.
Also, the comment "timeout behavior are consistent with other APIs" is no longer accurate after this change: this call now uses a custom 60-second timeout vs. the shared 10-second default. Suggested update:
* Uses shared `client` so token and Sentry headers are consistent with other APIs.
* A custom 60-second timeout is applied because Scratch conversion is significantly slower than typical API calls.
There was a problem hiding this comment.
@xgopilot Draft a pull request to go-wyvern:convert-timeout to fix comment here.
There was a problem hiding this comment.
Summary
Fixed the JSDoc comment in spx-gui/src/apis/sb2xbp.ts:
- Corrected endpoint path:
/scratch-convert→/util/sb2xbp - Updated timeout description to accurately reflect the custom 60-second timeout
Permission Required
Hi @nighca! I've prepared the changes but need permission to push to the fork.
View Details & Solution
Target: go-wyvern/builder (fork)
Push blocked:
remote: Permission to go-wyvern/builder.git denied to fennoai[bot].
fatal: unable to access 'https://github.com/go-wyvern/builder.git/': The requested URL returned error: 403
Recommended Solution: Install GitHub App
- Install xgopilot
- Select repository:
go-wyvern/builder - Click "Install"
- Return here and comment
There was a problem hiding this comment.
The fix is prepared but I couldn't push to go-wyvern/builder — the GitHub App (fennoai[bot]) doesn't have write access to that fork.
@xgopilot Push it to a temporary branch for goplus/builder instead of branch for go-wyvern/builder.
There was a problem hiding this comment.
Created PR #3092 with the JSDoc fixes for sb2xbp.ts:
- Corrected endpoint path:
/scratch-convert→/util/sb2xbp - Updated timeout description to reflect the custom 60-second timeout (no longer claims timeout behavior is consistent with other APIs)
@go-wyvern can cherry-pick or copy the change from that PR into the convert-timeout branch.
No description provided.