search

rombert / ereviewboard

A mylyn-based Eclipse integration for Review Board
46 stars 31 forks source link

"Invalid JSON Object" error truncation isn't detailed enough #105

Open lucianocheng opened 12 years ago

lucianocheng commented 12 years ago

Hey,

We often have issues with our firewalls / servers that cause ereviewboard to pass XML / HTML style errors when a request fails on a 500.

Line 117 of RestfulReviewboardReader.java truncates these error pages to 200 characters, which often isn't enough to read the full error message: https://github.com/rombert/ereviewboard/blob/master/org.review_board.ereviewboard.core/src/org/review_board/ereviewboard/core/client/RestfulReviewboardReader.java#L117

According to commit 9208496928e8d06dd88d039fe105259ae1b1b8c8, the truncation was put in place as part of the initial error handling of this page: https://github.com/rombert/ereviewboard/commit/9208496928e8d06dd88d039fe105259ae1b1b8c8

I would request that we remove the 200 character truncation to assist in diagnosing server error messages. It may cause the errors to be more verbose and ugly :/, but I think the utility of getting the entire message outweighs this.

If you approve, I'll create a pull request.

Thanks!

rombert commented 12 years ago

Sure, submit a pull request with whatever works for you.

Hey,

We often have issues with our firewalls / servers that cause ereviewboard to pass XML / HTML style errors when a request fails on a 500.

Line 117 of RestfulReviewboardReader.java truncates these error pages to 200 characters, which often isn't enough to read the full error message: https://github.com/rombert/ereviewboard/blob/master/org.review_board.ereviewboard.core/src/org/review_board/ereviewboard/core/client/RestfulReviewboardReader.java#L117

According to commit 9208496928e8d06dd88d039fe105259ae1b1b8c8, the truncation was put in place as part of the initial error handling of this page: https://github.com/rombert/ereviewboard/commit/9208496928e8d06dd88d039fe105259ae1b1b8c8

I would request that we remove the 200 character truncation to assist in diagnosing server error messages.  It may cause the errors to be more verbose and ugly :/, but I think the utility of getting the entire message outweighs this.

If you approve, I'll create a pull request.

Thanks!

Reply to this email directly or view it on GitHub: https://github.com/rombert/ereviewboard/issues/105

Sent from my (old) computer