jprichardson / node-klaw

A Node.js file system walker with a Readable stream interface. Extracted from fs-extra.
MIT License
317 stars 41 forks source link

fix potential stop-on-error behavior #32

Closed dzaman closed 1 year ago

dzaman commented 5 years ago

In #23, @dzek69 mentioned that klaw stops on error, and I encountered the same issue.

This behavior exists because the implementation of readable._read(size) necessitates a call to readable.push() before it will be called again, and it's possible for klaw to return from _read without calling push when it encounters an error. When this occurs, _read is never called again.

from the Stream documentation: Once the readable._read() method has been called, it will not be called again until the readable.push() method is called.

I solved this problem by more closely aligning the _read implementation with the suggestion in the docs:

from the Stream documentation: _read() should continue reading from the resource and pushing data until readable.push() returns false

With this PR, _read will continue to iterate over paths until push returns false or it runs out of paths. When an error is encountered before we call push, it will continue processing and make subsequent calls to push if there are any eligible paths, so errors no longer terminate the crawl prematurely.

Some notes:

RyanZim commented 5 years ago

This significantly modifies almost the entire codebase. I'm not a fan of directly inlining asyncToGenerator; I'd sooner just drop Node 6 support. I'm not extremely familiar with the details of this codebase, so naturally I'm hesitant to accept such drastic changes.

RyanZim commented 1 year ago

This PR is conflicted and outdated; closing.