1334: Fragment expansion brings up 404 error

th***@google.g********* (Google Code) (Is this you? Claim this profile.)
chipx86
chipx86
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.
chipx86
#1 chipx86
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
#2 th***@google.g********* (Google Code) (Is this you? Claim this profile.)
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.
chipx86
#3 chipx86
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
#4 ciao****@gmai***** (Google Code) (Is this you? Claim this profile.)
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.
chipx86
#5 chipx86
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
#6 HooHa*****@gmai***** (Google Code) (Is this you? Claim this profile.)
I see this same behavior in release 1.1 Alpha 1.  Was something not integrated?  
chipx86
#7 chipx86
Alpha 1 predates the 1.0.x fix for this. Please try a nightly.