Bug 201110

Summary: Phabricator: Raw diff reviews & creation of revisions is broken
Product: Services Reporter: Sean Bruno <sbruno>
Component: Code ReviewAssignee: Eitan Adler <eadler>
Status: Closed FIXED    
Severity: Affects Some People CC: araujo, eadler, emaste, gnn, koobs, mat, phabric-admin
Priority: --- Keywords: needs-qa, regression
Version: unspecified   
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D2579
Bug Depends on: 201121    
Bug Blocks:    
Attachments:
Description Flags
full `arc diff create` trace none

Description Sean Bruno freebsd_committer freebsd_triage 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 freebsd_triage 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.