Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetBodyStream closes io.Reader - not idiomatic? #1957

Open
KernelDeimos opened this issue Feb 14, 2025 · 1 comment
Open

SetBodyStream closes io.Reader - not idiomatic? #1957

KernelDeimos opened this issue Feb 14, 2025 · 1 comment

Comments

@KernelDeimos
Copy link

KernelDeimos commented Feb 14, 2025

I opened an issue on fiber when I thought something was missing, and eventually found out there was no bug or missing feature but rather very surprising behavior - I passed an io.Reader to c.SendStream (in fiber) and fasthttp actually type-asserts to io.ReadCloser and closes it for me.

This isn't idiomatic right? My understanding was whoever opens a resource is responsible for closing it, which is why we pass io.Reader instead of io.ReadCloser. While it's difficult for me to think of a practical example, in theory someone might not want fasthttp to close the stream.

I'm not pushing for any fix or change, as changing this would introduce regressions at a wide scale; maybe something to consider for a v3 release but mostly I'd really like to know if there is (or can be) a hard rule for this sort of thing.

@erikdubbelboer
Copy link
Collaborator

Agreed that this is not idiomatic and wouldn't happen if the library was designed right now. But as you also said it's not possible to change now without breaking backwards compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants