Repository navigation
Fix process.stdout exiting before done - #45
Conversation
| app.dataSource('db', { connector: 'memory' }); | ||
| var User = loopback.createModel('User'); | ||
| app.model(User, { dataSource: 'db' }); | ||
|
|
There was a problem hiding this comment.
I had to add something to the app template so my stdout buffer is long enough to have this error, open to suggestions of alternative ways to achieve this.
There was a problem hiding this comment.
This is fine. Can you please add a comment explaining that this is needed in order to have the script long enough to trigger output truncation?
|
Hi @sam-github, would you know if theres any implication on strong-agent if i do |
| process.nextTick(function() { | ||
| // previously used process.nextTick, but in Node 6 is causing | ||
| // exit to happen before finish writing leading to missing data | ||
| process.stdout.once('drain', function() { |
There was a problem hiding this comment.
what about stderr?
https://nodejs.org/docs/latest-v0.12.x/api/all.html#all_process_exitcode is better to use, but not if you need 0.10 support
There was a problem hiding this comment.
- We need to support 0.10
- We also need to force the process to exit. The reason for that is that some LoopBack connectors open a connection pool at start, and this pool blocks the app from exiting "naturally".
what about stderr?
Good point.
There was a problem hiding this comment.
@sam-github what about i revert to use nextTIck then add process.stdout._handle.setBlocking(true);?
maybe a more direct way to solve the problem to make sure behavior is the same
d8cef6f to
8b5bee9
Compare
| function runGenerator() { | ||
| // setting stdout to blocking so process.nextTick does not get interupted | ||
| // while stdout.write isn't finished in child_process on node v6 | ||
| process.stdout._handle.setBlocking(true); |
There was a problem hiding this comment.
doesn't work on 0.10 and 0.12, is that OK? As long as you make sure it only occurs for the generator and doesn't effect any other sub-commands, fine by me.
|
@slnode test please |
1 similar comment
|
@slnode test please |
fab4bdb to
8374967
Compare
|
@bajtos PTAL cc/ @sam-github |
| function waitForEvent(obj, name) { | ||
| return new Promise(function(resolve, reject) { | ||
| obj.on(name, resolve); | ||
| obj.on('error', reject); |
There was a problem hiding this comment.
I think .once is more appropriate both here and on the previous line too.
|
One nit-pick, the rest LGTM. No more reviews are necessary as far I am concerned. |
In Node v6 process.nextTick exit causes long messages from stdout.write to exit before the data is finished writing
8374967 to
6a8c379
Compare
|
@slnode test please |
In Node v6 process.nextTick exit causes long messages from stdout.write
to exit before the data is finished writing, looks like only happens in child_process.
@bajtos PTAL
cc/ @0candy