I may be misunderstanding something, but looking a...

# apollo-kotlin

[Stylianos Gakis]

I may be misunderstanding something, but looking at the function fun DefaultUpload.Builder.content(file: File): DefaultUpload.Builder here it seems like the line file.source().buffer() shouldn’t be there? Not an okio expert so bear with me 😄
We were using this function at some place where we forgot to migrate to File.toUpload() and we were experiencing that empty files were being sent over to the backend. And from my understanding I guessed that it probably happens from reading its contents somewhere since they’re readable only once. And in this function its seems like we’re reading the entire file into a buffer effectively emptying it out for the next time we do buffer() again, so that is why it’s then empty. After we migrated to File.toUpload() it seems like our files get sent to the backend properly. Anyone more proficient with okio can chime in to help me understand? 😊

[mbonnin]

Oh gosh. That should definitely not be there...

[mbonnin]

It shouldn't cause an empty file to be sent though. Worst case it's leaking a file descriptor

[Stylianos Gakis]

Is it just that line that needs to be removed? I can contribute it if so 😄

And yeah the more I look at it, you’re right that should not be causing at empty file, it’s just that we’re buffering the file itself twice which as you say it’s just some extra work that’s wasted 🤔

[mbonnin]

It's not even buffering the whole file, it's opening a BufferedSource but shouldn't read anything

[mbonnin]

So all in all it shouldn't be catastrophic but it's definitely not good. A PR to remove this is very welcome 🙏

[Stylianos Gakis]

Yeah I think I get it now. Which is bad (for me) since that means I don’t get why we’re getting an empty file uploaded 😂
Opened this for now, if I figure out what’s up and it has anything to do with the library and not me being silly I’ll mention it here 😄

[mbonnin]

Thank you 🙏

[Stylianos Gakis]

I think this bug was gonna drive me insane 😂
The PR is here https://github.com/HedvigInsurance/android/pull/1170/files (Specifically the ChatRepository) and it now works 😵‍💫

Okay tried with some more alternatives, you were right the extra buffering in there was not affecting anything.
I tried both approaches by inlining the functions to see what exactly was their difference and it literally was (aside from the extra buffer of course) just that one was setting the file name .fileName(file.name). When I added that myself it simply worked. It seems like this has more to do with our backend then, since the file name should be possible to be null.

So tl;dr I think it was our backend giving us the wrong error making me think we were sending an empty file, but the problem was somewhere else.

False alarm, but at least we got rid of one line of code so that’s always positive 😂

[mbonnin]

Cosmic ray 😅

[mbonnin]

Thanks for finding this in all cases, file descriptor leaks are never a good thing