Don't pass `--admin-host` to the Aphlict server
AbandonedPublic
Actions

Authored by joshuaspence on Jan 18 2015, 8:25 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Summary

Currently, I am unable to start the Aphlict server on our install:

> sudo su -c './bin/aphlict debug' aphlict
Starting Aphlict server in foreground...
Launching server:

    $ 'nodejs' '/usr/src/phabricator/support/aphlict/server/aphlict_server.js' '--client-port=' '--admin-port=22281' '--admin-host=phabricator.example.com' '--log=/mnt/logs/phd/aphlict.log'

[Sun Jan 18 2015 15:37:17 GMT-0500 (EST)] Started Server (PID 30693)
[Sun Jan 18 2015 15:37:17 GMT-0500 (EST)] 
<<< UNCAUGHT EXCEPTION! >>>

Error: listen EADDRNOTAVAIL
    at errnoException (net.js:904:11)
    at Server._listen2 (net.js:1023:19)
    at listen (net.js:1064:10)
    at net.js:1146:9
    at asyncCallback (dns.js:68:16)
    at Object.onanswer [as oncomplete] (dns.js:121:9)
>>> Server exited!

The issue is that we shouldn't be passing --admin-host=phabricator.example.com to the Node.js Aphlict server.

Test Plan

Applied this patch to our production install and was able to start the Aphlict server again.

Diff Detail

Repository

rP Phabricator

Branch

master

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 3896
Build 3908: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence updated this revision to Diff 27441.Jan 18 2015, 8:25 PM

joshuaspence retitled this revision from to Don't pass `--admin-host` to the Aphlict server.

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJan 18 2015, 8:25 PM

Herald added subscribers: epriestley, Korvin. · View Herald Transcript

Also fix client port

joshuaspence updated this object.Jan 18 2015, 8:43 PM

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added inline comments.Jan 18 2015, 8:45 PM

src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php
152–155	This fix is unrelated, but I snuck it into this diff.

joshuaspence added a child revision: D11359: Add `notification.bind-uri` to configure Aphlict server.Jan 18 2015, 8:46 PM

Fix default bindin

This opens the administrative port undesirably on most installs (where it is set to "localhost", which works fine), right? I'm not sure this is correct. It's desirable to listen only on localhost for the administrative port in most cases.

If I understand correctly, the current code would only work for single-host installs (where notification.server-uri is set to localhost). Furthermore, there is no way for me to make this work on my (multi-host) install because passing --admin-host=phabricator.example.com to aphlict_server.js doesn't work as expected.

I was contemplating adding a --admin-host flag (similar to D11288), but I decided against this in favor of D11359.