Calling CancelFunc in context doesn't cancel download

[maddie]

@oliverpool As per discussed in PR #73, creating a request with req.WithContext(ctx) that uses a context created from context.WithCancel and calling the returned CancelFunc doesn't stop the file from downloading.

The file being downloaded is around 160mb, which takes ~18s to download with 100Mbps connection.

Network activity can still be observed after calling CancelFunc, and the code doesn't return context.ErrCanceled.

[oliverpool]

I investigated this further and this seems strange, because the request should be cancelled.

There is even a test for that case : https://github.com/cavaliercoder/grab/blob/9eb8b4e4157e18f8abcd3684d5ebe359bf8c7868/client_test.go#L407-L441

Could you provide a complete code example where this is not working as expected?

[maddie]

Sorry for the late reply.

Here's a short repro example:

func main() {
    // download a 5.97GB file
    req, _ := grab.NewRequest(".", downloadUrl)
    ctx, cancel := context.WithCancel(context.Background())
    defer cancel()

    req.WithContext(ctx)
    resp := grab.DefaultClient.Do(req)

    t := time.NewTicker(200 * time.Millisecond)
    defer t.Stop()

    // channel for manual cancelling
    c := make(chan struct{})
    go func() {
        // simulating manual cancel in 2 seconds
        time.Sleep(2 * time.Second)
        c <- struct{}{}
    }()

    // channel for stop printing waiting dots
    wait := make(chan struct{})
Loop:
    for {
        select {
        case <-resp.Done:
            fmt.Println("Done")
            break Loop
        case <-t.C:
            fmt.Printf("%d / %d @ %.2f bps\n", resp.BytesComplete(), resp.Size, resp.BytesPerSecond())
        case <-c:
            // manual cancel here
            fmt.Println("Manual cancel")

            // print some dots to indicate waiting period after calling cancel
            go func() {
                for {
                    select {
                    case <-wait:
                        return
                    default:
                        fmt.Print(".")
                        time.Sleep(time.Second)
                    }
                }
            }()

            cancel()
            break Loop
        }
    }

    if err := resp.Err(); err != nil {
        // should print canceled error
        fmt.Printf("Received error: %s\n", err)
    } else {
        fmt.Println("Done successfully")
    }

    close(wait)
}

Output:

19399416 / 5969176217 @ 0.00 bps
39394144 / 5969176217 @ 0.00 bps
59450920 / 5969176217 @ 0.00 bps
80849024 / 5969176217 @ 0.00 bps
101886080 / 5969176217 @ 0.00 bps
121297024 / 5969176217 @ 72813456.22 bps
140462208 / 5969176217 @ 72813456.22 bps
160209024 / 5969176217 @ 72813456.22 bps
180324480 / 5969176217 @ 72813456.22 bps
200046720 / 5969176217 @ 72813456.22 bps
Manual cancel
..........................................................Done successfully

I think it should print the Received error line?

[oliverpool]

Your code has a small bug: WithContext returns a new request (it does not modify the given object, similar to context.WithCancel).

req = req.WithContext(ctx)

[maddie]

Ahh! That makes total sense. It works as expected now after the fix.

Thanks! I'll close this issue now.