1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00
Commit graph

86 commits

Author SHA1 Message Date
epriestley
99cbc20778 Reduce the verbosity of the "Aphlict" log
Summary:
See PHI1692. Currently, the Aphlict log is ridiculously verbose. As an initial pass at improving this:

  - When starting in "debug" mode, pass "--debug=1" to Node.
  - In Node, separate logging into "log" (lower-volume, more-important messages) and "trace" (higher-volume, less-important messages).
  - Only print "trace" messages in "debug" mode.

Test Plan: Ran Aphlict in debug and non-debug modes. Behavior unchanged in debug mode, but log has more sensible verbosity in non-debug mode.

Differential Revision: https://secure.phabricator.com/D21115
2020-04-14 13:24:44 -07:00
Alex Vandiver
11f1c13915 Append the intermediate chain to the "cert" parameter in Aphlict
Summary:
Per the documentation[1], any intermediate chain is to be
appended to the "cert" parameter.  The "ca" parameter controls the
root CA used to authenticate the client certificate, if one is
provided, and is not used for intermediate certificate chains -- nor
has it ever been.  It is not clear how this could have worked in the
past[2].

[1] https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options
[2] D15709

Test Plan:
Before this diff, with node 4.2.6 from Ubuntu packages:
```
$ openssl s_client -connect phabricator.dropboxer.net:22280 -verify 5 -CApath /etc/ssl/certs/
verify depth is 5
CONNECTED(00000003)
depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net
verify error:num=20:unable to get local issuer certificate
verify return:1
depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net
verify error:num=27:certificate not trusted
verify return:1
depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net
verify error:num=21:unable to verify the first certificate
verify return:1
---
Certificate chain
 0 s:/C=US/ST=California/L=San Francisco/O=Dropbox, Inc/OU=Dropbox Ops/CN=phabricator.dropboxer.net
   i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA
```

After:
```
$ openssl s_client -connect phabricator.dropboxer.net:22280 -verify 5 -CApath /etc/ssl/certs/
verify depth is 5
CONNECTED(00000003)
depth=2 C = US, O = DigiCert Inc, OU = www.digicert.com, CN = DigiCert High Assurance EV Root CA
verify return:1
depth=1 C = US, O = DigiCert Inc, OU = www.digicert.com, CN = DigiCert SHA2 High Assurance Server CA
verify return:1
depth=0 C = US, ST = California, L = San Francisco, O = "Dropbox, Inc", OU = Dropbox Ops, CN = phabricator.dropboxer.net
verify return:1
---
Certificate chain
 0 s:/C=US/ST=California/L=San Francisco/O=Dropbox, Inc/OU=Dropbox Ops/CN=phabricator.dropboxer.net
   i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA
 1 s:/C=US/ST=California/L=San Francisco/O=Dropbox, Inc/OU=Dropbox Ops/CN=phabricator.dropboxer.net
   i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA
 2 s:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert SHA2 High Assurance Server CA
   i:/C=US/O=DigiCert Inc/OU=www.digicert.com/CN=DigiCert High Assurance EV Root CA
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D18181
2018-06-26 19:34:28 +00:00
Chad Little
3b14c1fdd1 Update AphlictClientServer to support ws2 or ws3
Summary: This lets us support either ws2 or ws3. Fixes T12755

Test Plan: Update server to version 3, send message, watch debug log. Downgrade to 2.x, send messages, watch debug log. Everything seems OK

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12755

Differential Revision: https://secure.phabricator.com/D18411
2017-08-11 15:17:07 -07:00
epriestley
7fd98a5e86 Every so often, ask the Aphict server how things are going
Summary: Ref T12573. This sends a "ping" to the server, and a "pong" back to the client, every 15 seconds. This tricks ELBs into thinking we're doing something useful and productive.

Test Plan: Ran `bin/aphlict debug`, loaded Phabricator, saw ping/pong in logs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12573

Differential Revision: https://secure.phabricator.com/D17717
2017-04-17 20:33:43 -07:00
epriestley
02194f0fc8 After Aphlict reconnects, ask the server to replay recent messages
Summary:
Fixes T12563. If we've ever seen an "open", mark all future connections as reconnects. When we reconnect, replay recent history.

(Until duplicate messages (T12564) are handled better this may cause some notification duplication.)

Also emit a reconnect event (for T12566) but don't use it yet.

Test Plan: {F4912044}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12563

Differential Revision: https://secure.phabricator.com/D17708
2017-04-17 15:54:51 -07:00
epriestley
88157a9442 Hold recent messages in Aphlict so they can be replayed after clients reconnect
Summary:
Ref T12563. Before broadcasting messages from the server, store them in a history buffer.

A future change will let clients retrieve them.

Test Plan:
  - Used the web frontend to look at the buffer, reloaded over time, sent messages. Saw buffer size go up as I sent messages and fall after 60 seconds.
  - Set size to 4 messages, sent a bunch of messages, saw the buffer size max out at 4 messages.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12563

Differential Revision: https://secure.phabricator.com/D17707
2017-04-17 15:53:58 -07:00
epriestley
19c6ccb279 Allow bin/aphlict to start without a valid database connection
Summary:
Ref T11818. See that task for a description.

This is like a tiny pinch of hackiness but not really unreasonable. Basically, `aphlict` is already an "uber-server" and one copy can handle a ton of instances.

Test Plan: Started `bin/aphlict` without a valid database connection.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11818

Differential Revision: https://secure.phabricator.com/D16854
2016-11-13 16:43:23 -08:00
epriestley
bbb321395a Support Aphlict clustering
Summary:
Ref T6915. This allows multiple notification servers to talk to each other:

  - Every server has a list of every other server, including itself.
  - Every server generates a unique fingerprint at startup, like "XjeHuPKPBKHUmXkB".
  - Every time a server gets a message, it marks it with its personal fingerprint, then sends it to every other server.
  - Servers do not retransmit messages that they've already seen (already marked with their fingerprint).
  - Servers learn other servers' fingerprints after they send them a message, and stop sending them messages they've already seen.

This is pretty crude, and the first message to a cluster will transmit N^2 times, but N is going to be like 3 or 4 in even the most extreme cases for a very long time.

The fingerprinting stops cycles, and stops servers from sending themselves copies of messages.

We don't need to do anything more sophisticated than this because it's fine if some notifications get lost when a server dies. Clients will reconnect after a short period of time and life will continue.

Test Plan:
  - Wrote two server configs.
  - Started two servers.
  - Told Phabricator about all four services.
  - Loaded Chrome and Safari.
  - Saw them connect to different servers.
  - Sent messages in one, got notifications in the other (magic!).
  - Saw the fingerprinting stuff work on the console, no infinite retransmission of messages, etc.

(This pretty much just worked when I ran it the first time so I probably missed something?)

{F1218835}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6915

Differential Revision: https://secure.phabricator.com/D15711
2016-04-14 13:26:30 -07:00
epriestley
07fc8f17cc Support "ssl.chain" in Aphlict configuration
Summary: Fixes T10806. Although browsers don't seem to care about this, it's more correct to support it, and the new test console uses normal `cURL` and does care.

Test Plan:
  - Hit the error case for providing a chain but no key/cert.
  - Used `openssl s_client -connect localhost:22280` to connect to local Aphlict servers.
  - With SSL but no chain, saw `openssl` fail to verify the remote.
  - With SSL and a chain, saw `openssl` verify the identify of the remote.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10806

Differential Revision: https://secure.phabricator.com/D15709
2016-04-14 10:41:21 -07:00
epriestley
d4bf2a147b Make paths and Aphlict instance names less ambiguous
Summary:
Fixes T10783 (what little of it remains). Ref T10697.

Aphlict currently uses request paths for two different things:

  - multi-tenant instancing in the Phacility cluster (each instance gets its own namespace within an Aphlict server);
  - some users configure nginx and apache to do proxying or SSL termination based on the path.

Currently, these can collide.

Put a "~" before the instance name to make it unambiguous. At some point we can possibly just use a GET parameter, but I think there was some reason I didn't do that originally and this sequence of changes is disruptive enough already.

Test Plan: Saw local Aphlict unambiguously recognize "local.phacility.com" as instance "local", with a "~"-style URI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697, T10783

Differential Revision: https://secure.phabricator.com/D15705
2016-04-14 04:57:21 -07:00
epriestley
2930733ac9 Complete modernization of Aphlict configuration
Summary:
Fixes T10697. This finishes bringing the rest of the config up to cluster power levels.

Phabricator is now given an arbitrarily long list of notification servers.

Each Aphlict server is given an arbitrarily long list of ports to run services on.

Users are free to make them meet in the middle by proxying whatever they want to whatever else they want.

This should also accommodate clustering fairly easily in the future.

Also rewrote the status UI and changed a million other things. 🐗

Test Plan:
{F1217864}

{F1217865}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15703
2016-04-14 04:57:00 -07:00
epriestley
c6b0925954 Move Aphlict logging and PID configuration options to config file
Summary: Ref T10697. Mostly straightforward. Also allow the server to have multiple logs and log options in the future (e.g., different verbosities or separate admin/client logs or whatever). No specific plans for this, but the default log is pretty noisy today.

Test Plan: Set up a couple of logs, started server, saw it log to them.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15702
2016-04-14 04:55:19 -07:00
epriestley
c84dee522b Move server-related Aphlict options to a configuration file
Summary: Ref T10697. This isn't everything but starts generalizing options and moving us toward a cluster-ready state of affairs.

Test Plan: Started server in various configurations, hit most (all?) of the error cases with bad configs, sent test notifications.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15701
2016-04-14 04:54:42 -07:00
epriestley
e32ce529d7 Begin generalizing Aphlict server to prepare for clustering/sensible config file
Summary:
Ref T10697. Currently, `aphlict` takes a ton of command line flags to configure exactly one admin server and exactly one client server.

I want to replace this with a config file. Additionally, I plan to support:

  - arbitrary numbers of listening client ports;
  - arbitrary numbers of listening admin ports;
  - SSL on any port.

For now, just transform the arguments to look like they're a config file. In the future, I'll load from a config file instead.

This greater generality will allow you to do stuff like run separate HTTP and HTTPS admin ports if you really want. I don't think there's a ton of use for this, but it tends to make the code cleaner anyway and there may be some weird cross-datacneter cases for it. Certainly, we undershot with the initial design and lots of users want to terminate SSL in nginx and run only HTTP on this server.

(Some sort-of-plausible use cases are running separate HTTP and HTTPS client servers, if your Phabricator install supports both, or running multiple HTTPS servers with different certificates if you have a bizarre VPN.)

Test Plan: Started Aphlict, connected to it, sent myself test notifications, viewed status page, reviewed logfile.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15700
2016-04-14 04:54:20 -07:00
Joshua Spence
368f359114 Use PhutilClassMapQuery instead of PhutilSymbolLoader
Summary: Use `PhutilClassMaQuery` instead of `PhutilSymbolLoader`, mostly for consistency. Depends on D13588.

Test Plan: Poked around a bunch of pages.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13589
2015-08-14 07:49:01 +10:00
Joshua Spence
36e2d02d6e phtize all the things
Summary: `pht`ize a whole bunch of strings in rP.

Test Plan: Intense eyeballing.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12797
2015-05-22 21:16:39 +10:00
epriestley
ebcab8edb6 Namespace Aphlict clients by request path, plus other fixes
Summary:
Fixes T7130. Fixes T7041. Fixes T7012.

Major change here is partitioning clients. In the Phacility cluster, being able to get a huge pile of instances on a single server -- without needing to run a process per instance -- is desirable.

To accomplish this, just bucket clients by the path they connect with. This will let us set client URIs to `/instancename/` and then route connections to a small set of servers. This degrades cleanly in the common case and has no effect on installs which don't do instancing.

Also fix two unrelated issues:

  - Fix the timeouts, which were incorrectly initializing in `open()` (which is called during reconnect, causing them to reset every time). Instead, initialize in the constructor. Cap timeout at 5 minutes.
  - Probably fix subscriptions, which were using a property with an object definition. Since this is by-ref, all concrete instances of the object share the same property, so all users would be subscribed to everything. Probably.

Test Plan:
  - Hit notification status page, saw version bump and instance/path name.
  - Saw instance/path name in client and server logs.
  - Stopped server, saw reconnects after 2, 4, 16, ... seconds.
  - Sent test notification; received test notification.
  - Didn't explicitly test the subscription thing but it should be obvious by looking at `/notification/status/` shortly after a push.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7041, T7012, T7130

Differential Revision: https://secure.phabricator.com/D11769
2015-02-16 11:31:15 -08:00
Pierre Moreau
172566a769 Aphlict - fix incrementation of _messagesIn
Summary: Ref T7124. The local version of `this` in the handler of 'end' was incremented rather than the global one.

Test Plan: Sending test notifications did not increment the `messages.in` value before this patch even if it should have. With the patch sending test notifications does increment `messages.in`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7124

Differential Revision: https://secure.phabricator.com/D11648
2015-02-03 08:02:23 -08:00
Pierre Moreau
a424fec7bb Aphlict - fix getActiveListenerCount return value
Summary: Ref T7126. Dictionaries do not have a `length` property unlike arrays resulting in the `getActiveListenerCount()` function returning undefined results. Using the `length` property on the array of keys will work.

Test Plan: Using `wscat` to generate multiple connections to the server, establish new ones and close others while keeping an eye on the displayed `clients.active` value.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7126

Differential Revision: https://secure.phabricator.com/D11647
2015-02-03 07:01:49 -08:00
Pierre Moreau
4af1fd2a79 Aphlict - remove listeners when clients close the connection
Summary: Ref T7110. Listeners are now removed when clients close the connection to avoid stacking a never ending number of unused listeners.

Test Plan: Using `wscat` to connect to the Aphlict server; when closing the connection a 'Diconnected.' will appear in the logs and the number of active listeners is decreased by one.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7110

Differential Revision: https://secure.phabricator.com/D11634
2015-02-02 14:57:20 -08:00
Joshua Spence
04ee853cec Add the logger earlier in the Aphlict startup process
Summary: Add the logger as soon as possible so that the log file will contain errors if the `ws` module cannot be loaded.

Test Plan: Ran `./bin/aphlict debug` without having the `ws` module installed. Saw errors in the logs.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11457
2015-01-22 07:45:55 +11:00
Joshua Spence
f61846b469 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.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6998

Differential Revision: https://secure.phabricator.com/D11456
2015-01-22 06:46:49 +11:00
Joshua Spence
27422ffe8e Use single quotes in JavaScript files
Summary: Use single quotes to keep JSHint happy.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11431
2015-01-20 08:53:47 +11:00
Joshua Spence
53834d1471 Enable "strict" mode for NodeJS
Summary:
In particular, this changes the behavior of NodeJS in the following ways:

- Any attempt to get or modify the global object will result in an error.
- `null` values of `this` will no longer be evaluated to the global object and primitive values of this will not be converted to wrapper objects.
- Writing or deleting properties which have there writeable or configurable attributes set to false will now throw an error instead of failing silently.
- Adding a property to an object whose extensible attribute is false will also throw an error now.
- A functions arguments are not writeable so attempting to change them will now throw an error `arguments = [...]`.
- `with(){}` statements are gone.
- Use of `eval` is effectively banned.
- `eval` and `arguments` are not allowed as variable or function identifiers in any scope.
- The identifiers `implements`, `interface`, `let`, `package`, `private`, `protected`, `public`, `static` and `yield` are all now reserved for future use (roll on ES6).

Test Plan: Verified that Aphlict was still functional.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11430
2015-01-20 07:43:10 +11:00
epriestley
4636833f3d Fix module imports in Aphlict server
Summary:
This was broken in D11383. Basically, I had the `ws` module installed globally whilst testing, but the changes made do not work if the `ws` module is installed locally (i.e. in the `./support/aphlict/server/node_modules` directory). After poking around, it seems that this is due to the sandboxing that is done by `JX.require`.

A quick fix is to just //not// use `JX.require`, although you may have a better idea?

The error that is occurring is as follows:

```
<<< UNCAUGHT EXCEPTION! >>>

Error: Cannot find module 'ws'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at extra.require (/usr/src/phabricator/webroot/rsrc/externals/javelin/core/init_node.js:48:16)
    at /usr/src/phabricator/support/aphlict/server/lib/AphlictClientServer.js:10:17
    at Script.(anonymous function) [as runInNewContext] (vm.js:41:22)
    at Object.JX.require (/usr/src/phabricator/webroot/rsrc/externals/javelin/core/init_node.js:58:6)
    at Object.<anonymous> (/usr/src/phabricator/support/aphlict/server/aphlict_server.js:102:4)
    at Module._compile (module.js:456:26)
>>> Server exited!
```

Test Plan: Now able to start the Aphlict server.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: Korvin, epriestley

Maniphest Tasks: T6987

Differential Revision: https://secure.phabricator.com/D11425
2015-01-19 11:46:14 -08:00
Joshua Spence
2b12f61602 Don't exit from the Aphlict Server prematurely
Summary: By calling `process.exit(1)` we are forcing the Node.js process to exit prematurely. Specifically, the process is terminated before the error is written to the log file. This can be verified by inspecting the value of `debug._logs[0]._writableState.writing` immediately before the process is terminated.

Test Plan: Ran `./bin/aphlict debug` without the `ws` module being installed. Verified that `<<< UNCAUGHT EXCEPTION! >>>` was echoed to the console as well as the log file. Also verified that the exit status was non-zero.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11426
2015-01-20 06:36:51 +11:00
Joshua Spence
9a7ad972cd Refactoring of the Aphlict server
Summary: Tidy the Aphlict server by splitting the functionality into two main modules, `AphlictClientServer` and `AphlictAdminServer. There is still further tidying that could be done here, but I feel that this puts us in a much better place.

Test Plan: Sent notifications via `/notification/status/`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11383
2015-01-19 07:49:50 +11:00
Joshua Spence
270a0c54b4 Fix permissions on Aphlict log
Summary: Currently, the Aphlict server created the log file (if it doesn't exist) but then immediately fails with "Unable to open logfile". It seems that we don't set the permissions correctly.

Test Plan: Deleted log file and was able to start the Aphlict server.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11351
2015-01-13 08:26:04 +11:00
epriestley
0d070c91dc Fix Aphlict logging
Summary:
Yeahhhhhhhh....

  - Open a "stream", not a "steam".
  - Make error easier for users to understand.
  - Write to the log in debug mode so the issue is more apparent.

Test Plan:
  - Started server with bad permissions, got usable error message.
  - Started server with good permissions, got logfile.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11339
2015-01-12 08:16:08 -08:00
Joshua Spence
638cf20c9d Allow the Aphlict server to bind to localhost
Summary: If you are running the Aphlict server behind a reverse proxy (such as `nginx`) then there's no need to bind to `0.0.0.0`. Add a `--client-host` flag to `aphlict_server.js` to allow binding to a different hostname. Also changed the other flags for consistency and clarity.

Test Plan: Started, stopped and debug the Aphlict server.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11288
2015-01-09 11:10:47 +11:00
epriestley
f0ade1be1c Stop advising users to use npm -g to install websockets
Summary: Fixes T6910. This advice is bad, doesn't work, and was based on me havng an outdated or incorrect understanding of Node and npm.

Test Plan: Read documentation.

Reviewers: richardvanvelzen, btrahan, chad, joshuaspence

Reviewed By: chad, joshuaspence

Subscribers: epriestley

Maniphest Tasks: T6910

Differential Revision: https://secure.phabricator.com/D11285
2015-01-08 14:02:14 -08:00
Joshua Spence
a87f2cd610 Fix --ssl-cert for Aphlict
Summary: `PhabricatorAphlictManagementWorkflow` passes `--ssl-cert` but `aphlict_server.js` expects `--ssl-certificate`.

Test Plan: Tested on a production system.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11283
2015-01-09 08:51:47 +11:00
epriestley
9e0f70e17d Rewrite Aphlict to use Websockets
Summary:
Fixes T6559. No more flash, use Websockets. This is less aggressive than the earlier version, and retains more server logic.

  - Support "wss".
  - Make the client work.
  - Remove "notification.user" entirely.
  - Seems ok?

Test Plan:
In Safari, Firefox and Chrome, saw the browsers connect. Made a bunch of comments/updates and saw notifications.

Notable holes in the test plan:

  - Haven't tested "wss" yet. I'll do this on secure.
  - Notifications are //too fast// now, locally. I get them after I hit submit but before the page reloads.
  - There are probably some other rough edges, this is a fairly big patch.

Reviewers: joshuaspence, btrahan

Reviewed By: joshuaspence, btrahan

Subscribers: fabe, btrahan, epriestley

Maniphest Tasks: T6713, T6559

Differential Revision: https://secure.phabricator.com/D11143
2015-01-08 10:03:00 -08:00
Joshua Spence
87f11a091d Minor improvements to handling Aphlict status code
Summary: Self explanatory.

Test Plan: `curl`ed a few URLs.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11147
2015-01-03 09:11:08 +11:00
Joshua Spence
9f31e023f4 Minor improvements for handling of /status/ for Aphlict
Summary: We don't need to handle any request data for the `/status/` route, so we can simplify this code slightly.

Test Plan:
```lang=bash
> curl http://127.0.0.1:22281/status/
{"uptime":2543,"clients.active":0,"clients.total":0,"messages.in":0,"messages.out":0,"log":"/var/log/aphlict.log","version":6}
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11145
2015-01-03 09:10:49 +11:00
Joshua Spence
44198d68a7 Use the correct Content-Type for the Aphlict status route
Summary: The `/status/` route for the Aphlict server returns JSON.

Test Plan:
```lang=bash
> curl -I http://127.0.0.1:22281/status/
HTTP/1.1 200 OK
Content-Type: application/json
Date: Fri, 02 Jan 2015 13:08:34 GMT
Connection: keep-alive
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11144
2015-01-03 09:09:48 +11:00
Joshua Spence
a00efcbfca Fix some functions being used before they are defined
Summary: Minor JSHint fixes.

Test Plan: `arc lint`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11073
2014-12-30 22:09:53 +11:00
Joshua Spence
fcc7dbbf15 Fix a few minor JSHint warnings
Summary: Basically, don't assign expressions to a variable if the variable is unused.

Test Plan: `arc lint`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11072
2014-12-30 03:02:58 -08:00
Joshua Spence
f2fee5a84e Return a HTTP 500 instead of a HTTP 400 if an internal error occurs in the Aphlict server
Summary: Ref T5651. Only throw a HTTP 400 if the data is invalid (i.e. the request is bad). If something bad happens when trying to transmit the notification, throw a HTTP 500 instead.

Test Plan: Eye-ball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5651

Differential Revision: https://secure.phabricator.com/D9968
2014-07-18 09:20:00 +10:00
Joshua Spence
41a8837f78 Make HTTP errors returned from the Aphlict server more specific
Summary: Ref T5651. Currently, the Aphlict server returns either `200 OKAY` or `400 Bad Request`. We could return more specific errors in some cases and this may assist with debugging.

Test Plan:
Sent myself a test notification at `/notification/status/` and saw the Aphlict server process the request (running in debug mode). Also poked around with `curl`:

```
> curl http://localhost:22281/
405 Method Not Allowed

> curl http://localhost:22281/ -d ""
400 Bad Request

> curl http://localhost:22281/foobar/
404 Not Found
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5651

Differential Revision: https://secure.phabricator.com/D9967
2014-07-18 09:01:46 +10:00
epriestley
6bf4ec97d5 Fix HTTP 400 from notification server for JSON subscription objects
Summary: Fixes T5651. Sometime we'll send an object to the notification server for `subscribers`, which it will choke on. Use `array_values()` to make sure we're sending an array.

Test Plan: With `(object)` instead, got a consistent error ("no .filter method on object"). With `array_values()`, no error.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5651

Differential Revision: https://secure.phabricator.com/D9963
2014-07-17 14:48:54 -07:00
Joshua Spence
7304e29dec Various minor JSHint fixes.
Summary: Various fixes as suggested by JSHint.

Test Plan: Eye-balled it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9783
2014-07-01 06:00:12 +10:00
epriestley
79366795e7 React to Aphlict disconnects in the UI
Summary: Ref T5365. Surface disconnects in the UI.

Test Plan:
  - Connected, then killed the server.
  - Saw disconnected event and appropriate update in the UI.

{F169605}

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5365

Differential Revision: https://secure.phabricator.com/D9706
2014-06-24 09:41:40 -07:00
epriestley
76cefde0b3 Show Aphlict connection status in notification menu
Summary:
Fixes T5373. Ref T5281. Several changes:

  - The `marshallExceptions` thing is useful if JS throws an exception when invoked from Flash, so set it. The resulting exceptions are a little odd (not escaped correctly, e.g.) but way better than nothing.
  - Put connection status in the notification menu.
  - When the connection fails, try to provide contextual help where we can.

Test Plan: {F169493}

Reviewers: chad, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5281, T5373

Differential Revision: https://secure.phabricator.com/D9700
2014-06-23 16:26:16 -07:00
epriestley
dd91732df3 Make JX.Aphlict a real singleton with a more sensible initialization order
Summary:
Ref T5373. The control flow between `aphlict-listener` and `JX.Aphlict` is pretty weird right now, where the listener (which is the highest-level component) has intimate knowledge of how to put the SWF on the page.

Instead:

  - Make `JX.Aphlict` a real singleton.
  - Instantiate it sooner.
  - Have it handle the flash setup handshake.

Test Plan: Loaded page in debug mode, saw normal flow take place.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5373

Differential Revision: https://secure.phabricator.com/D9699
2014-06-23 15:19:34 -07:00
epriestley
80f26e96ea Install an uncaught exception handler in Aphlict
Summary:
Ref T5373. This seems to work pretty much correctly.

Also stop popping bubbles and just use the log, since users find the bubbles confusing/not useful and they're not great for developers either.

Future diffs will expose more user-facing stuff.

Test Plan: Added `throw` to AphlictClient.as, got a log in the parent window.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5373

Differential Revision: https://secure.phabricator.com/D9698
2014-06-23 15:18:36 -07:00
Joshua Spence
97bb13d1d7 Discover workflows automatically.
Summary: Most scripts detect the relevant workflows automatically. Some scripts, however, use a hardcoded list of workflows.

Test Plan: Ran `./bin/aphlict`, `./bin/cache` and `./bin/phd`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9564
2014-06-16 09:00:25 +10:00
Joshua Spence
279a0e5371 Don't explicitly keep track of _activeListenerCount in the Aphlict server.
Summary: The `_activeListenerCount` variable is overkill, we should be able to achieve the same result using `Object.keys(this._listeners).length`.

Test Plan:
Mucked around in a NodeJS shell.

```lang=js
> Object.keys({}).length
0
> Object.keys({foo: 'bar'}).length
1
> Object.keys({1: 'foo', 2: 'bar'}).length
2
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9554
2014-06-16 05:20:00 +10:00
Joshua Spence
e40b18fb75 Fix call to debug.log.
Summary: As pointed out by @epriestley in D9458#62, this call to `debug.log` is missing an argument.

Test Plan: meh..

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D9485
2014-06-14 11:28:28 -07:00
Joshua Spence
cbd95b91b5 Implement unsubscription in the Aphlict client.
Summary: Ref T5284. When an `AphlictClient` is closed, it will eventually be purged from the pool by the `AphlictMaster`. When this happens, also unsubscribe the purged client from all notifications, and send an `unsubscribe` command to the Aphlict server if possible.

Test Plan:
Verified the output of the Aphlict server (running in debug mode):

```
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:192.168.1.1
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Connected from ::ffff:192.168.1.1
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"subscribe","data":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Subscribed to: ["PHID-USER-cb5af6p4oepy5tlgqypi"]
[Wed Jun 11 2014 23:21:39 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"subscribe","data":["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]}
[Wed Jun 11 2014 23:21:39 GMT+0000 (UTC)] <Listener/2> Subscribed to: ["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]
[Wed Jun 11 2014 23:21:57 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"unsubscribe","data":["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]}
[Wed Jun 11 2014 23:21:57 GMT+0000 (UTC)] <Listener/2> Unsubscribed from: ["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5284

Differential Revision: https://secure.phabricator.com/D9492
2014-06-11 20:02:01 -07:00