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

Consume STDERR from ripgrep to allow indexing process to complete #406

Merged
merged 2 commits into from Nov 19, 2019

Conversation

@fredden
Copy link
Contributor

fredden commented Nov 2, 2019

Description of the Change

When indexing a project with a lot of broken symbolic links, the indexing process never completes. This is because the ripgrep process has written more data to its error output (STDERR) than the calling process has buffer to receive; until this buffer is cleared, the process will not terminate. The solution to this is to process the data on the receiving side.

This pull request adds a simple no-op handler for anything sent back on STDERR from ripgrep.

Alternate Designs

I considered logging the error output from ripgrep. This produced a lot of noise when running the test-suite and is unlikely to be of use for anyone in the real world. Logging the error messages returned from ripgrep is a trivial change.

Benefits

Indexing process always completes.

Possible Drawbacks

Diagnosing some issues may be difficult due to the lack of logging, however before this change any messages sent to STDERR were already being lost.

Applicable Issues

No obviously related open issues in this repository at time of opening this pull request. Lots of seemly-related external results when searching for this problem on the web.

fredden added 2 commits Nov 2, 2019
When ripgrep produces a lot of output on STDERR, the indexing process never
completes as the process is waiting to have its buffer read before terminating.
Confirmed that test fails before previous commit is applied.
@rsese

This comment has been minimized.

Copy link
Member

rsese commented Nov 5, 2019

Thanks for the contribution! CI failure looks unrelated so we'll ask the other maintainers to take a look.

@darangi darangi self-assigned this Nov 19, 2019
@darangi

This comment has been minimized.

Copy link
Contributor

darangi commented Nov 19, 2019

Thanks for the contributions 🙇 @fredden

@darangi darangi merged commit 717de04 into atom:master Nov 19, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@fredden fredden deleted the fredden:stderr-stall-results branch Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.