Context-bound RPC requests #1133

New issue

Open

opened 2025-12-28 17:15:21 +00:00 by sami · 2 comments

sami commented

2025-12-28 17:15:21 +00:00

Owner

Originally created by @roman-khimov on GitHub (May 30, 2023).

We have a notion of client-wide context.Context, but it's not very useful in most cases, contexts are more useful in per-request setting where they limit individual request processing time. Problems:

net/http provides NewRequestWithContext for that, but we're not using it currently
WSClient uses a library that doesn't have any notion of contexts (they can be emulated though, we're waiting on internal channels anyway)
the biggest one -- API. We've had the API we have for very long time and it's kinda OK in that it works, so we can either break all of it or provide some additional set of methods or provide some *Client.WithRequestContext(ctx) *Client to be used for that purpose like smth, err := c.WithRequestContext(ctx).GetApplicationLog(...)

CC @smallhive, @notimetoname.

Originally created by @roman-khimov on GitHub (May 30, 2023). We have a notion of client-wide `context.Context`, but it's not very useful in most cases, contexts are more useful in per-request setting where they limit individual request processing time. Problems: * `net/http` provides [NewRequestWithContext](https://pkg.go.dev/net/http#NewRequestWithContext) for that, but we're not using it currently * `WSClient` uses a library that doesn't have any notion of contexts (they can be emulated though, we're waiting on internal channels anyway) * the biggest one -- API. We've had the API we have for very long time and it's kinda OK in that it works, so we can either break all of it or provide some additional set of methods or provide some `*Client.WithRequestContext(ctx) *Client` to be used for that purpose like `smth, err := c.WithRequestContext(ctx).GetApplicationLog(...)` CC @smallhive, @notimetoname.

sami added the

rpc

enhancement

labels

2025-12-28 17:15:21 +00:00

sami commented

2025-12-28 17:15:21 +00:00

Author

Owner

@notimetoname commented on GitHub (May 30, 2023):

Some time ago one good guy opened my eyes and I started to see dirty code in a stored (nested inside structs) context.Context; it was kinda obvious but I saw so many lines of code that do not follow that simple rule so I had not noticed that before. My life will never be the same. I would also add a linter (not a "classic" one but neither an article is) that checks and prevents it.

net/http provides NewRequestWithContext for that

IIRC, go devs admitted it is a mistake (for backward compatibility) and nothing can be done now.

smth, err := c.WithRequestContext(ctx).GetApplicationLog(...)

Look weird to me. I would only go that way if I was sure I need to keep all the old code working.

@notimetoname commented on GitHub (May 30, 2023): Some time ago one good guy opened my eyes and I started to see dirty code in a stored (nested inside structs) `context.Context`; it was kinda obvious but I saw so many lines of code that do not follow that simple rule so I had not noticed that before. My life will never be the same. I would also add [a linter](https://github.com/sivchari/containedctx) (not a "classic" one but neither an [article](https://go.dev/blog/context-and-structs) is) that checks and prevents it. > `net/http` provides [NewRequestWithContext](https://pkg.go.dev/net/http#NewRequestWithContext) for that IIRC, go devs admitted it is a mistake (for backward compatibility) and nothing can be done now. > `smth, err := c.WithRequestContext(ctx).GetApplicationLog(...)` Look weird to me. I would only go that way if I was sure I _need_ to keep all the old code working.

sami commented

2025-12-28 17:15:21 +00:00

Author

Owner

@roman-khimov commented on GitHub (Jun 4, 2025):

The other option is to just provide a new client and deprecate the old one. They can share a lot of code internally yet the new API would be clearly different from the old one and everyone could migrate as needed.

@roman-khimov commented on GitHub (Jun 4, 2025): The other option is to just provide a new client and deprecate the old one. They can share a lot of code internally yet the new API would be clearly different from the old one and everyone could migrate as needed.