1334: Fragment expansion brings up 404 error
- Fixed
- Review Board
| th***@google.g********* (Google Code) (Is this you? Claim this profile.) | |
 |
|
| Oct. 2, 2009 |
What version are you running? 1.0.3.1 What's the URL of the page containing the problem? https://reviewboard.<domain>/r/<reviewid>/diff/#index_header What steps will reproduce the problem? 1. Submit a review request through Post Review 2. In draft mode, click on "View Diff" once 3. Publish the review request 4. Open the request (either as submitter or reviewer) 5. Click on "View Diff" 6. Try to expand expand a hidden source code section by clicking on "Expand" What is the expected output? What do you see instead? Clicking on any of the "Expand" hyperlinks (to expand collapsed source code sections) in the RB diff viewer brings up a 404 error. The URL to retrieve the file fragment uses an incorrect diff index (0 instead of 1), such as: https://reviewboard.<domain>/r/<reviewid>/diff/0/fragment/2785/chunk/8/ Manually invoking the URL with a corrected diff # correctly retrieves the fragment. What operating system are you using? What browser? Server: Windows Server 2003. Client: browser-independent. Please provide any additional information below. The issue only occurs when selecting "View Diff" at least once in draft mode and does not occur when blindly publishing the review request.
Interesting. Now, what happens if you repro this and then, on the diff where it normally fails, you shift-reload to bypass the cache? I suspect this is either a server-side or a client-side caching problem, so that'll be an important test.
-
+ NeedInfo -
+ Component-DiffViewer
The following test was performed with a web browser that did not previously access
the review request, so client-side caching should not be in place.
When initially accessing the request and clicking "Expand" the error happened again.
Just to be on the safe side I also shift-reloaded the page and hit "Expand" again -
with unchanged results.
One of the "Expand" links reads:
-- 8< --
87 lines hidden [<a href="#" onclick="javascript:expandChunk('file0', '2830', '0',
null, '0', this); return false;">Expand</a>]
-- 8< --
It apparently references the incorrect diff revision. In the same HTTP response
(towards the top of the request) there is also a variable that correlates to the diff
revision:
var gRevision = '1';
This one, however, seems to be correct and reference diff #1, so I'd rule out any
HTTP-related caching issues.
Next, I restarted the Apache httpd that hosts Review Board to rule out any in-memory
mod_python issues. The issue still persisted.
Restarting memcached, however, solved the issue, so cached content seems to play an
important role.
Looks like we're not including the revision number, just the database ID of the filediff. That would explain it. I'll get this fixed in a point release.
-
- NeedInfo + Confirmed -
- Priority-Medium + Priority-Critical + Milestone-Release1.0.x + EasyFix -
+ chipx86
We're seeing this same issue. Looking forward to the fix! Just as an added piece of info... turning off memcache on the server resolves the 404 error for us. Restarting memcache also helps in specific cases, but other folks were continuing to run into 404 errors so I've had to turn it off completely for now. Strange.
Should be fixed in our git repository now. You can try doing an install from there. Fixed on release-1.0.x (rdbc7668) and master (r52dc35f).
-
- Confirmed + Fixed
I see this same behavior in release 1.1 Alpha 1. Was something not integrated?
Alpha 1 predates the 1.0.x fix for this. Please try a nightly.