repo.unpack callback is never called using HTTP transport

[whyleee]

Tested in my code and your clone-test repo: TCP transport works well, but HTTP transport freezes at 99% and never calls the callback. It actually receives and saves all the data, but somehow doesn't call the callback after that. I tried to debug, but it's hard for me to understand where the problem is.
Note: I use this code for Node.js HTTPS request implementation, because XHR doesn't work with GitHub. But I don't think this code is related to the problem.
Could you please run the code in your clone-test repo and see if you have the same problem with cloning HTTPS urls? Or maybe you have ideas why the callback is not called?

[techniq]

@creationix any ideas?

[creationix]

Sorry, I haven't looked at this in quite a while. I do remember that clone-test was working over XHR (as long as I installed the browser extension to inject the CORS headers). It is possible that your code has a bug though I can't see it right away.

I've never seen that trick of using Buffer.concat in the data loop (I usually build a normal array and then concat once inside end), but I think your style should work fine as well.

It's also possible that something in js-git broke since I was testing clone-test. Maybe compare the last date I pushed to clone-test with the date of the last js-git and related publishes?

[jauco]

I can reproduce this bug. @whyleee it seems you have fixed this bug in whyleee/nogit#7 what was the fix?

[jauco]

It might have broken when you switched to culvert, or it might only break on certain data streams.

What happens is that at the end the onSave method calls channel.take(onRead) when the channel is empty. (https://github.com/creationix/js-git/blob/master/mixins/pack-ops.js#L129 ) This puts the onRead callback on the readQueue in culvert (https://github.com/creationix/culvert/blob/master/channel.js#L59 ). However the readQueue will not be touched until someone adds some new data to the stream (which won't happen because all data was already pushed to the stream).

It looks like I need to add a method to culvert to close a channel right? and then have the users of culvert use that method. @creationix is that correct?

[creationix]

Sounds good I think. Like I said before, I haven't touched this code in years.

[whyleee]

@jauco I haven't really fixed this issue, I just used regex for progress output to find the last chunk 😄 whyleee/nogit@8dfcb01#diff-f8fba8b03f3acaae3ecdcdbb453e0d7d

[fabrixxm]

worked after I changed line 92 in lib/pack-codec.js:

- if (!state) return;
+ if (!state) { if(checksum.length===40) emit(); return; }

It works, but could be wrong...