From 41a8837f78dbe739c000ac0c2fda67d7e3b6a1fe Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Fri, 18 Jul 2014 09:01:46 +1000 Subject: [PATCH] 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 --- .../client/PhabricatorNotificationClient.php | 2 + support/aphlict/server/aphlict_server.js | 56 ++++++++++--------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/applications/notification/client/PhabricatorNotificationClient.php b/src/applications/notification/client/PhabricatorNotificationClient.php index f7d45316b4..dedbda6d70 100644 --- a/src/applications/notification/client/PhabricatorNotificationClient.php +++ b/src/applications/notification/client/PhabricatorNotificationClient.php @@ -40,6 +40,8 @@ final class PhabricatorNotificationClient { private static function postMessage(array $data) { $server_uri = PhabricatorEnv::getEnvConfig('notification.server-uri'); + $server_uri = id(new PhutilURI($server_uri)) + ->setPath('/'); id(new HTTPSFuture($server_uri, json_encode($data))) ->setMethod('POST') diff --git a/support/aphlict/server/aphlict_server.js b/support/aphlict/server/aphlict_server.js index f0bc7b854c..f079a059bd 100644 --- a/support/aphlict/server/aphlict_server.js +++ b/support/aphlict/server/aphlict_server.js @@ -163,33 +163,39 @@ var start_time = new Date().getTime(); var receive_server = http.createServer(function(request, response) { // Publishing a notification. - if (request.method == 'POST') { - var body = ''; + if (request.url == '/') { + if (request.method == 'POST') { + var body = ''; - request.on('data', function(data) { - body += data; - }); + request.on('data', function(data) { + body += data; + }); - request.on('end', function() { - try { - var msg = JSON.parse(body); + request.on('end', function() { + try { + var msg = JSON.parse(body); - debug.log('notification: ' + JSON.stringify(msg)); - ++messages_in; - transmit(msg); + debug.log('notification: ' + JSON.stringify(msg)); + ++messages_in; + transmit(msg); - response.writeHead(200, {'Content-Type': 'text/plain'}); - } catch (err) { - debug.log( - '<%s> Bad Request! %s', - request.socket.remoteAddress, - err); - response.statusCode = 400; - response.write('400 Bad Request'); - } finally { - response.end(); - } - }); + response.writeHead(200, {'Content-Type': 'text/plain'}); + } catch (err) { + debug.log( + '<%s> Bad Request! %s', + request.socket.remoteAddress, + err); + response.statusCode = 400; + response.write('400 Bad Request\n'); + } finally { + response.end(); + } + }); + } else { + response.statusCode = 405; + response.write('405 Method Not Allowed\n'); + response.end(); + } } else if (request.url == '/status/') { request.on('data', function() { // We just ignore the request data, but newer versions of Node don't @@ -212,8 +218,8 @@ var receive_server = http.createServer(function(request, response) { response.end(); }); } else { - response.statusCode = 400; - response.write('400 Bad Request'); + response.statusCode = 404; + response.write('404 Not Found\n'); response.end(); } }).listen(config.admin, config.host);