fix(gateway): remove buffer-peek-stream #2227
Conversation
This removes buffer-peek-stream which caused uncaught errors inside of Hapijs and replaces it with much simpler approach to set content-type header in Gateway responses. Closes libp2p/js-libp2p#374 Closes libp2p/pull-mplex#13 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
License: MIT Signed-off-by: Marcin Rataj <[email protected]>
|
This looks like a good alternative. Do we have Gateway tests we can add for the early hangups from clients? If we're going to deploy any js-ipfs nodes we should get more tests around abusive client behavior. |
|
@jacobheun not at the moment: gateway tests in js-ipfs skip HTTP transport and inject request directly to hapijs using provided DSL: Lines 342 to 345 in 01419cb I agree end-to-end tests like one you described are needed, but I'd argue its true not only for js-ipfs. AFAIK we don't have gateway interop test suite, but creating one could be Q3 OKR under "gateway as a product" or web browser maintenance work. Having that kicked off, I'll be happy to seed the interop test suite with tests for issue at hand and other things (I've already started collecting potential issues around HTTP headers in ipfs/in-web-browsers#132) |
|
Note: @lidel is setting up tests for gateway interop here ipfs/interop#76 |
|
@lidel is it possible to test this fix using the |
|
@alanshaw I tried briefly, but was unable to reproduce it without spawning real server. That is why I'd rather have it in ipfs/interop#76 |
This is an alternative to #2227 that does not hit datastore twice. The underlying error was caused by multiple PassThrough/pipe calls. Turns out go-ipfs does not compress anything by default, so we can remove PassThrough responsible for streaming of compressed payload and fix the issue while keeping optimization introduced by buffer-peek-stream. Closes libp2p/js-libp2p#374 Closes libp2p/pull-mplex#13 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
This is an alternative to #2227 that does not hit datastore twice. The underlying error was caused by multiple PassThrough/pipe calls. Turns out go-ipfs does not compress anything by default, so we can remove PassThrough responsible for streaming of compressed payload and fix the issue while keeping optimization introduced by buffer-peek-stream. Closes libp2p/js-libp2p#374 Closes libp2p/pull-mplex#13 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
This PR fixes "random" daemon crashes with
NodeError: Cannot call write after a stream was destroyed(details in libp2p/js-libp2p#374)It removes
buffer-peek-streamwhich caused uncaught errors inside of Hapijs when HTTP client closed HTTP connection before receiving entire payload from a Gateway port.After falling into rabit hole of accounting for how Hapijs handles Node streams I decided to take step back, change approach and replace stream peeking with much simpler code that does not introduce intermediate streams.
Steps to reproduce: libp2p/js-libp2p#374 (comment)
Closes libp2p/js-libp2p#374
Closes libp2p/pull-mplex#13