Add support for context.Context

[ewohltman]

I was considering a way to incorporate the usage of context.Context so that if a timeout I set when handling an event is exceeded, I can have finer grain control over canceling requests within a logical context and cleaning up the associated resources.

Simplified example:

// VoiceStateUpdate is the callback function for the VoiceStateUpdate event from Discord
func (config *Config) VoiceStateUpdate(s *discordgo.Session, vsu *discordgo.VoiceStateUpdate) {
    ctx, cancelCtx := context.WithTimeout(context.Background(), 30 * time.Second)
    defer cancelCtx()

    user, err := s.UserWithContext(ctx, vsu.UserID)
    if err != nil {
        // Log err
        return
    }

    // Numerous additional API calls
}

The idea being that if the time to make the numerous API calls add up to exceed the timeout in my context before I finish the VoiceStateUpdate method, I would like to return from the method early.

Currently, the built-in *http.Client in discordgo.Session has an overall timeout set, but that governs each individual request. The usage of context.Context gives us a higher-level control over a logical grouping of requests.

[NaniteFactory]

I was also looking for something like this. My bot has hundreds of requests being made concurrently and blocked by the rate limiter. I want to throw away some of the least important requests to stop them bottleneck other requests, but can’t find a way. Every request made by (*Session) should really be cancelable with context.Context.

[orthoplex64]

The pull request was closed without comment; why was it closed? I too was looking for these "WithContext" variants of functions when I was reading the DiscordGo docs for the first time.

[ewohltman]

@CarsonHoffman There might be demand for this, and context.Context is low level enough that discordgo should support it.

[CarsonHoffman]

I'm more than happy to consider a solution that integrates contexts into the library; I'm not sure that the proposed solution was the best for overall maintainability and surface area of the API. Having something emulating net.http.(*Request).WithContext would be interesting, as it would allow it to be applied to any request with one additional method. I'm not sure how feasible this would be with the shared, concurrent nature of Session, but it might be something interesting to explore.

[ewohltman]

I think the tougher part is that Session is a long-living object whereas http.Request is generally a short-lived object where using contexts make sense (e.g. we might want to cancel an HTTP request, but probably not cancel the entire Session). To continue with the example of http.Request, the standard library increased their API surface area by implementing a new method as well: http.NewRequest was the original (and is still there) and the new http.NewRequestWithContext. Digging into the source of them, http.NewRequest now calls http.NewRequestWithContext and provides context.Background for backwards compatibility.

// NewRequest wraps NewRequestWithContext using the background context.
func NewRequest(method, url string, body io.Reader) (*Request, error) {
	return NewRequestWithContext(context.Background(), method, url, body)
}

The original PR followed that example from the standard library. Fortunately, I still have my fork and I've been keeping it up to date with master if we want to look at it again :+1:

[jalavosus]

Any chance this could be revived?