fix: decouple pin polling context from upload context deadline#99
Conversation
The upload handler wraps the entire upload+pin flow in a single 5-minute context. By the time waitForPin starts polling, most of the deadline is already consumed. When the parent context expires, in-flight GetOperation HTTP calls are cancelled mid-flight, producing context deadline exceeded. Create a fresh context with the configured upload timeout for all pin operations (ListOperations, WaitForOperation, and pin verification retry). Pass explicit poll options to WaitForOperation instead of relying on SDK defaults that race against the parent context.
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
Code Coverage ReportTotal Coverage: 53.6% Generated from commit: 90b968c |
|
@kody review |
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
|
@kody review |
This comment has been minimized.
This comment has been minimized.
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
|
@kody review |
Address Kody review: context.Background() ignored parent cancellation (Ctrl+C/SIGINT). Use a goroutine that only propagates context.Canceled, not deadline expiry, so pin polling gets a fresh full timeout while user cancellation still aborts it. Add test verifying user cancellation propagates to pin polling.
|
Addressed Kody's review in The fix now distinguishes between user cancellation (Ctrl+C/SIGINT,
Added a second test case ( |
This comment has been minimized.
This comment has been minimized.
|
@kody review |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
1 participant
Summary
ListOperations,WaitForOperation, and pin verification retry) instead of reusing the upload context that may have already consumed most of its deadlineWithPollInterval,WithPollTimeout) toWaitForOperationinstead of relying on SDK defaults that race against the parent contextSummary
This PR fixes an issue where pin polling could fail prematurely because it reused the upload context, whose deadline may have been nearly exhausted by the time pinning started. Since server-side pin processing (e.g., DAG block downloads) can take longer than the upload itself, the inherited deadline was too short.
Changes
waitForPinnow creates a fresh context with the configured upload timeout instead of reusing the parent upload context, preventing premature cancellation when the upload context deadline is nearly consumed.WaitForOperationis now called with a 2-second poll interval and the upload timeout as the poll timeout, making polling behavior explicit and consistent.waitForPin(operation status checks and pin status verification retries) to use the new fresh context.