Bug 201110 - Phabricator: Raw diff reviews & creation of revisions is broken
Summary: Phabricator: Raw diff reviews & creation of revisions is broken
Status: Closed FIXED
Alias: None
Product: Services
Classification: Unclassified
Component: Code Review (show other bugs)
Version: unspecified
Hardware: Any Any
: --- Affects Some People
Assignee: Eitan Adler
URL: https://reviews.freebsd.org/D2579
Keywords: needs-qa, regression
Depends on: 201121
Blocks:
  Show dependency treegraph
 
Reported: 2015-06-25 15:31 UTC by Sean Bruno
Modified: 2015-06-27 16:04 UTC (History)
7 users (show)

See Also:


Attachments
full `arc diff create` trace (2.15 KB, text/plain)
2015-06-26 03:00 UTC, Kubilay Kocak
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sean Bruno freebsd_committer 2015-06-25 15:31:04 UTC
When looking at raw diff reviews, sometimes I'm forwarded to blank pages and sometimes I get nginx errors.  Logs from phabric backend looks serious when I access the diffs.

https://reviews.freebsd.org/D2579

2015/06/24 15:23:51 [error] 51784#0: *28814 FastCGI sent in stderr:
"PHP message: [2015-06-24 15:23:51] EXCEPTION: (PhutilProxyException)
Invalid parameter information was passed to method
'differential.parsecommitmessage'. {>} (PhutilJSONParserException)
Parse error on line 1 at column 10: Invalid string, it appears you
forgot to terminated the string, or attempted to write a multiline
string which is invalid at [<phutil>/src/parser/PhutilJSONParser.php:32]
PHP message: arcanist(head=master, ref.master=9a7c4d87a850),
phabricator(head=master, ref.master=e25c6c0d363d), phutil(head=master,
ref.master=66de37985920)
PHP message:   #0 <#3> PhutilJSONParser::parse(string) called at
[<phutil>/src/utils/utils.php:1047]
PHP message:   #1 <#3> phutil_json_decode(string) called at
[<phabricator>/src/applications/conduit/controller/PhabricatorConduitAPI
Controller.php:629]
PHP message:   #2 <#2>
PhabricatorConduitAPIController::decodeConduitParams(AphrontRequest,
string) called at
[<phabricator>/src/applications/conduit/controller/PhabricatorConduitAPI
Controller.php:37]
PHP message:   #3 phlog(PhutilProxyException) called at
[<phabricator>/src/applications/conduit/controller/PhabricatorConduitAPI
Controller.php:111]
PHP message:   #4 PhabricatorConduitAPIController::processRequest()
called at [<phabricator>/src/aphront/AphrontController.php:33]
PHP message:   #5 AphrontController::handleRequest(AphrontRequest)
called at
[<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration
.php:226]
PHP message:   #6
AphrontApplicationConfiguration::processRequest(AphrontRequest,
PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at
[<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration
.php:140]
PHP message:   #7
AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink)
called at [<phabricator>/webroot/index.php:21]" while reading response
header from upstream, client: 127.0.1.2, server:
phabric.isc.freebsd.org, request: "POST
/api/differential.parsecommitmessage HTTP/1.0", upstream:

From nginx-acccess logs:
127.0.1.2 - - [24/Jun/2015:15:46:49 +0000] "GET
/file/data/wyo74wuc5e65vnncts64/PHID-FILE-mifzuh6knbacorxffzny/D2579.dif
f
HTTP/1.0" 200 0 "-" "Mozilla/5.0 (X11; FreeBSD amd64; rv:38.0)
Gecko/20100101 Firefox/38.0"
Comment 1 George V. Neville-Neil freebsd_committer 2015-06-25 15:33:37 UTC
Here is a pastebin of what I get on an arc diff --create

http://pastebin.com/pM7GpQsR
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2015-06-25 15:39:31 UTC
Assign to Eitan as he was/is taking care of this. CC'ing all people who have requested status updates either on IRC or mailing lists to date (that I've seen)
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2015-06-26 02:56:48 UTC
The symptoms I am observing are:

[user@CURRENT-amd64:/usr/home/koobs/repos/freebsd/ports] arc diff --create devel/pybugz
Exception
ERR-CONDUIT-CORE: Invalid parameter information was passed to method 'differential.getcommitmessage'.
(Run with `--trace` for a full exception trace.)
49%
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2015-06-26 03:00:13 UTC
Created attachment 158064 [details]
full `arc diff create` trace

Adding full arc diff --create --trace log
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2015-06-26 04:43:46 UTC
note that creation of diffs and missing diffs are two different issues:

(a) creation of diffs: This is
due to a bug in cURL:  cURL 7.43 has a bug where "Content-Length" is
incorrectly cached between requests on the same connection, which
breaks the 2nd..Nth request when connections are reused.  Recent
versions of libphutil work around the issue (the port update is here:
https://reviews.freebsd.org/D2894).  In the meantime either downgrade
cURL to a version earlier than 7.43.0, upgrade cURL to HEAD, or wait
for 7.43.1 and upgrade cURL.


(b) this was due a big fsck up on my part.  I believe I have fixed it (and random testing of various reported broken diffs seems to have confirmed it).

Marking the bug resolved due to (b) being resolved and (a) being worked on
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2015-06-26 09:41:20 UTC
Re-open until the fix lands in bug 201121, then its resolved
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2015-06-27 16:04:56 UTC
The blocking commit was closed.  Closing this bug.