Add support for using context.WithTimeout()

[Jacalz]

I am using wormhole-william for my own project and there it serves as a way of syncing data between two devices. I was trying to set a timeout because I don't users of my application to have the sending running forever if they start it. Right now the only way to stop it is to receive on a different application or kill the application.

Context values are already part of the function parameters that need to be used when calling the function, so the API would not need to change to accommodate this.

I would for example like to have this working:

package main

import (
    "context"
    "fmt"
    "log"
    "os"
    "time"

    "github.com/psanford/wormhole-william/wormhole"
)

func main() {
    if len(os.Args) < 2 {
        fmt.Fprintf(os.Stderr, "usage: %s <file>\n", os.Args[0])
        os.Exit(1)
    }

    filename := os.Args[1]

    f, err := os.Open(filename)
    if err != nil {
        log.Fatal(err)
    }

    var c wormhole.Client

    ctx, _ := context.WithTimeout(context.Background(), 10*time.Second)
    code, status, err := c.SendFile(ctx, filename, f)
    if err != nil {
        log.Fatal(err)
    }

    fmt.Println("On the other computer, please run: wormhole receive")
    fmt.Printf("Wormhole code is: %s\n", code)

    s := <-status

    if s.Error != nil {
        log.Fatalf("Send error: %s", s.Error)
    } else if s.OK {
        fmt.Println("OK!")
    } else {
        log.Fatalf("Hmm not ok but also not error")
    }
}

I would greatly appreciate seeing this added. It would be highly beneficial for my application and for a lot more people too if I guess. Having this implemented would also mean that context.WithDeadline() should work and possibly context.Cancel(), but I haven't tested that.

[psanford]

We should support this. Right now I think we support cancellation in places where the underlying API supports it (e.g. we call DialContext which means if you cancelled during a the connection establishment phase that would work). We don't handling cancelling during the streaming phase of the connection.

The main thing that needs to be fixed right now is in file_transport.go a context cancellation should trigger a close to be called on the connection object.

There's probably also issues cancelling in the rendezvous client that we should check for.

[psanford]

This should be fixed now.

[Jacalz]

Amazing. Thanks you very much. Will make sure to update wormhole-gui to take advantage of these changes 🙂

[Jacalz]

FYI, it looks like this wasn’t part of the changelog on the GitHub release.

[psanford]

Yeah, I cut the release before this was merged. Will probably cut another release sometime soonish.

[Jacalz]

Ah, I see. For some reason, the release https://github.com/psanford/wormhole-william/releases/tag/refs%2Ftags%2Fv1.0.5 doesn’t show that there are more commits after it, while https://github.com/psanford/wormhole-william/releases/tag/v1.0.5 does.

[Jacalz]

I finally had the time to test this, but unfortunately this does not quite seem to be working yet when using the updated code on the master branch. Running the example code in the issue description does still not work. I think it might be because cancelling the send does not send something in the channel. I would have expected to see the log.Fatalf("Hmm not ok but also not error") to be printed when the send was cancelled.

[Jacalz]

I'm also getting this error when cancelling an in-progress send:

2021/04/02 17:54:10 Fyne error:  Error on sending file
2021/04/02 17:54:10   Cause: write tcp 192.168.1.220:39049->192.168.1.220:37624: use of closed network connection
2021/04/02 17:54:10   At: /home/jacob/go/src/github.com/Jacalz/wormhole-gui/internal/transport/bridge/send.go:110
panic: runtime error: index out of range [0] with length 0

goroutine 100 [running]:
github.com/Jacalz/wormhole-gui/internal/transport/bridge.(*SendList).OnFileSelect.func1.1(0xc0000bc000, 0x0, 0xa1ed20, 0xc000c74600, 0xc0000d0ca0)
        /home/jacob/go/src/github.com/Jacalz/wormhole-gui/internal/transport/bridge/send.go:93 +0xd2
github.com/Jacalz/wormhole-gui/internal/transport/bridge.(*SendList).OnFileSelect.func1(0xc0000bc000, 0xa1ed20, 0xc000c74600, 0xc0000d0ca0, 0xa1fcc8, 0xc0002443c0, 0x0)
        /home/jacob/go/src/github.com/Jacalz/wormhole-gui/internal/transport/bridge/send.go:116 +0x234
created by github.com/Jacalz/wormhole-gui/internal/transport/bridge.(*SendList).OnFileSelect
        /home/jacob/go/src/github.com/Jacalz/wormhole-gui/internal/transport/bridge/send.go:91 +0x16c

[psanford]

The initial changes I made were for in-progress transfers, not for a client waiting for the other side to connect to the mailbox. I've merged #50 which should address your example code.

It is expected that cancelling an in-progress transfer will result in an error. Ideally we would return the context cancellation error in this case instead of the network send error, but that isn't plumbed through right now.

[Jacalz]

Thank you very much. It is now working much more as expected. It would be great if those context cancellation errors could be plumbed though for some nicer close of the running transfers, but apart from that I am very satisfied with this :slightly_smiling_face: