Closed
Bug 559452
Opened 14 years ago
Closed 14 years ago
Improve the code and error checking from TinderboxPushlog Robot
Categories
(Tree Management Graveyard :: TBPL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
2.99 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
From Gerv in bug 557445 comment 54: > Ehsan: a quick code review of that code leads me to make the following > comments: > > - Please use a proper JSON escaping function. If your comment has a backslash > in, for example, the JSON will not be well-formed and BzAPI will probably fail. > > - If you just want a list of comments, don't get the whole bug with comments, > just get the comments: > https://wiki.mozilla.org/Bugzilla:REST_API:Methods#List_comments_for_bug_.28.2Fbug.2F.3Cid.3E.2Fcomment_GET.29 > > - Please check the return codes from the functions you call! This may well tell > you if things are failing or not! > > Looking at the logs on api-dev, the only errors in the last few days for > /latest are people doing big searches which time out. > > Gerv
Assignee | ||
Comment 1•14 years ago
|
||
Attachment #439116 -
Flags: review?(mstange)
Updated•14 years ago
|
Attachment #439116 -
Flags: review?(mstange) → review+
Comment 2•14 years ago
|
||
Comment on attachment 439116 [details] [diff] [review] Patch (v1) >+ // server returned an error >+ alert("Submitting the comment to bug " + id + " failed (" + result.error + ")"); result.error will always be "1" or a true value. For useful diagnostics, you want to print a serialization of the entire error object, with the possible exception of the html_page member: https://wiki.mozilla.org/Bugzilla:REST_API:Objects#Error Gerv
Assignee | ||
Comment 3•14 years ago
|
||
(In reply to comment #2) > (From update of attachment 439116 [details] [diff] [review]) > >+ // server returned an error > >+ alert("Submitting the comment to bug " + id + " failed (" + result.error + ")"); > > result.error will always be "1" or a true value. For useful diagnostics, you > want to print a serialization of the entire error object, with the possible > exception of the html_page member: > https://wiki.mozilla.org/Bugzilla:REST_API:Objects#Error No, result.error is the result of curl_errno, which may be 0 for no error, or an error code.
Assignee | ||
Comment 4•14 years ago
|
||
http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/8a50d4854cdc
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•14 years ago
|
||
I just solved the year's most stupid php bug! http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/af70100f64cd This newline character caused output, and would subsequently generate errors like: <br /> <b>Warning</b>: Cannot modify header information - headers already sent by (output started at /www/htdocs/tmstests/tinderboxpushlog/summaries/JSON.php:808) in <b>/www/htdocs/tmstests/tinderboxpushlog/submitBugzillaComment.php</b> on line <b>19</b><br /> Which resulted in the "Marking 1 bug..." prompt never disappearing. Markus, could you please deploy that changeset? Thanks!
Comment 6•14 years ago
|
||
Done. One way to avoid these kinds of gotchas is to omit the ?> at the end. It's not required as far as I know.
Comment 7•14 years ago
|
||
Ehsan: do let me know if the new code reveals that the BzAPI is erroring out for you when it shouldn't be :-) Gerv
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #6) > One way to avoid these kinds of gotchas is to omit the ?> at the end. It's not > required as far as I know. Hmm, good trick. I don't know if it's allowed, but the PHP parser can handle it for sure, and it's not like PHP is a standardized language anyway! (In reply to comment #7) > Ehsan: do let me know if the new code reveals that the BzAPI is erroring out > for you when it shouldn't be :-) Sure, I will. Thanks for your help!
Updated•10 years ago
|
Product: Webtools → Tree Management
Updated•9 years ago
|
Product: Tree Management → Tree Management Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•