Skip to content

Conversation

@admosity
Copy link

@admosity admosity commented Dec 3, 2015

It appears that the exit signal is fired before the entire stdout stream is flushed. This causes issues for editors/IDEs that listen for the process exit event (atom in my case):

Using this atom package: https://github.com/AtomLinter/linter-htmlhint

image

Early end of input:
image

Slapped on a quick fix to listen for the drain event for json (as that's what is used in the linter I am using).

@admosity
Copy link
Author

admosity commented Dec 3, 2015

I realized that this is not going to work because this has been switched over to streams. On the previous version, I hooked into formatResult. You're going to need a more complete solution across all the formatters.

@admosity
Copy link
Author

admosity commented Dec 3, 2015

Re-opening for exposure.

@yaniswang
Copy link
Contributor

Can you show me your node version?

@admosity
Copy link
Author

admosity commented Dec 4, 2015

It's version 0.12.7. I'm still doing some digging as to what exactly is causing this.

@yaniswang
Copy link
Contributor

In my case, exit signal after stdout, node version is v4.2.2

@admosity
Copy link
Author

admosity commented Dec 4, 2015

In my test the process exit event comes before the stdout close event.
However I was able to pull out all the data. As per the node child process
docs. The exit event doesn't mean that the stdout stream is finished. This
leads me to think that there is an issue with atom itself when an child
process exit event is received, because I saw the truncation of the stream
happen there when it didn't happen in similar code I had.

That being said, using the callback from the write call to stdout to ensure
the stream is flushed does fix the issue with the linter I was using. Check
the gist I have from the other referenced issue, I'd link it but I'm on my
phone. I have a quick fix for the json format which the linter uses.
On Dec 4, 2015 1:07 AM, "Yanis Wang" [email protected] wrote:

In my case, exit signal after stdout, node version is v4.2.2


Reply to this email directly or view it on GitHub
#101 (comment).

  console.log and console.log with formatter functionality
- added check to ensure writes have finished from the formatter
- `done` event emitted after formatter completed execution
@admosity
Copy link
Author

admosity commented Dec 5, 2015

I've added a more complete solution across all the formatters.

@yaniswang
Copy link
Contributor

Can you show a demo code to match this issue?

@admosity
Copy link
Author

admosity commented Dec 6, 2015

https://github.com/admosity/HTMLHint/blob/exit-test-case/test/executable.spec.js

The test I added is kind of spotty. I've gotten it to fail once on v0.9.12. It fails pretty consistently on atom:
image

And here it's working with the current pull request:
image

@yaniswang
Copy link
Contributor

@admosity can you upgrade your node to new version?

@yaniswang
Copy link
Contributor

11d10aa

I add the executable.spec tese case, and it run successed!

@thedaviddias thedaviddias added bug Functionality that does not work as intended/expected #status: need more info 👋 help wanted We are looking for community help labels Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Functionality that does not work as intended/expected help wanted We are looking for community help

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants