Page MenuHomePhabricator
Differential D11456

Fix Aphlict exit status
ClosedPublic
Actions

Authored by joshuaspence on Jan 21 2015, 7:34 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 12:06 AM
Unknown Object (File)
Sun, Nov 17, 2:59 PM
Unknown Object (File)
Thu, Nov 14, 10:09 PM
Unknown Object (File)
Mon, Nov 11, 6:54 PM
Unknown Object (File)
Mon, Nov 4, 5:57 AM
Unknown Object (File)
Mon, Nov 4, 5:52 AM
Unknown Object (File)
Mon, Nov 4, 5:52 AM
Unknown Object (File)
Sun, Nov 3, 6:56 AM
View All Files
Subscribers
epriestley
Korvin

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Maniphest Tasks
T6998: After removing `process.exit(1)`, Node exits with error code 0 after throwing
Commits
Restricted Diffusion Commit
rPf61846b46908: Fix Aphlict exit status
Summary

Fixes T6998. Based on https://groups.google.com/d/msg/nodejs/zF7GEoPccqw/n26c6gPaluwJ.

Test Plan

Ran Aphlict and forced an error. Saw error messages in the logs and also saw a non-zero exit status.

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3966
Build 3979: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence updated this revision to Diff 27541.Jan 21 2015, 7:34 AM
joshuaspence retitled this revision from to Fix Aphlict exit status.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
joshuaspence added a task: T6998: After removing `process.exit(1)`, Node exits with error code 0 after throwing.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJan 21 2015, 7:34 AM
Herald added subscribers: epriestley, Korvin. · View Herald Transcript
epriestley accepted this revision.Jan 21 2015, 2:44 PM
epriestley edited edge metadata.
epriestley added inline comments.
support/aphlict/server/aphlict_server.js
108

Do we need a return; here? This does not actually exit the process, does it?

Maybe renaming exit() to set_exit_code() or similar would be more clear.

This revision is now accepted and ready to land.Jan 21 2015, 2:44 PM
epriestley added a comment.Jan 21 2015, 2:44 PM

(In particular, I would expect --test to cause the server to start.)

joshuaspence added inline comments.Jan 21 2015, 7:40 PM
support/aphlict/server/aphlict_server.js
108

Ah yeah, that seems correct.

joshuaspence updated this revision to Diff 27554.Jan 21 2015, 7:45 PM
joshuaspence edited edge metadata.

Rename exit() to set_exit_code()

joshuaspence updated this revision to Diff 27555.Jan 21 2015, 7:47 PM

Fix --test mode

Closed by commit rPf61846b46908: Fix Aphlict exit status (authored by joshuaspence, committed by joshuaspence). · Explain WhyJan 21 2015, 7:47 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
support/
aphlict/
server/
9 lines

Diff 27541

View Options

support/aphlict/server/aphlict_server.js

Loading...
Phabricator · Built by Phacility