4524: TypeError: string indices must be integers

seb
brennie
brennie

What version are you running?

Version 2.5.9
The problem was not present in 2.5.8

What's the URL of the page containing the problem?

http://reviewboard.taitradio.net/r/new/

What steps will reproduce the problem?

  1. Click create new request
  2. Select a mercurial repository (subversion repos are fine)

What is the expected output? What do you see instead?

On the web page I see:

There was an error loading information from this repository:
HTTP 500 Internal Server Error
This may be a temporary failure. Try again

I also get an email the content of which I have attached.

Reviewboard does not crash, but the post from commit functionality obviously does not work.

What operating system are you using? What browser?

Ubuntu Xenial with updates
Chrome + FF

Please provide any additional information below.

Very likely related to https://hellosplat.com/s/beanbag/tickets/4032/

Traceback (most recent call last):
  File "/home/reviewboard/.venv/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 112, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/reviewboard/.venv/local/lib/python2.7/site-packages/django/views/decorators/cache.py", line 52, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/home/reviewboard/.venv/local/lib/python2.7/site-packages/django/views/decorators/vary.py", line 19, in inner_func
    response = func(*args, **kwargs)
  File "/home/reviewboard/.venv/local/lib/python2.7/site-packages/djblets/webapi/resources/base.py", line 196, in __call__
    request, method, view, api_format=api_format, *args, **kwargs)
  File "/home/reviewboard/.venv/local/lib/python2.7/site-packages/djblets/webapi/resources/mixins/api_tokens.py", line 65, in call_method_view
    return view(request, *args, **kwargs)
  File "/home/reviewboard/.venv/local/lib/
seb
#1 seb

BTW, pip freeze --local outputs:

appdirs==1.4.0
cffi==1.9.1
cryptography==1.7.2
Django==1.6.11
django-evolution==0.7.6
django-haystack==2.4.1
django-multiselectfield==0.1.5
django-pipeline==1.3.27
django-reset==0.2.0
Djblets==0.9.6
dnspython==1.15.0
docutils==0.13.1
enum34==1.1.6
feedparser==5.2.1
flup==1.0.3.dev20110405
futures==3.0.5
idna==2.2
ipaddress==1.0.18
Markdown==2.4.1
mercurial==4.1
mimeparse==0.1.3
olefile==0.44
packaging==16.8
paramiko==2.1.2
Pillow==4.0.0
pillowfight==0.3
pkg-resources==0.0.0
publicsuffix==1.1.0
pyasn1==0.2.2
pycparser==2.17
pycrypto==2.6.1
Pygments==2.2.0
pyparsing==2.1.10
python-dateutil==1.5
python-ldap==2.4.32
python-memcached==1.58
pytz==2016.10
recaptcha-client==1.0.6
ReviewBoard==2.5.9
six==1.10.0
subvertpy==0.9.2
Whoosh==2.7.4

chipx86
#2 chipx86

Would you mind gathering some information from your Mercurial repository for me?

First, what version of Mercurial are you running?

Can you look up the repository path for it and, in a browser, navigate to: <that-path>/json-log/?rev=branch(.)

(I believe that's the path it's looking up in this case -- may need to try with different values inside the branch().)

Can you then provide the output? If this repository contains confidential information, perhaps you could show the entry for some innocuous commit. You can also mark your attachment as private by clicking the lock icon at the top of your post when replying.

  • -New
    +NeedInfo
  • +Release-2.5.x
  • -Priority:Medium
    +Component:HostingServices
    +Priority:High
    +SCM:Mercurial
seb
#3 seb

On the HG server we are running mercurial 3.7.3-1ubuntu1.
RB itself (the client) uses (I believe) the pip installed version in the venv. Pip shows that as being at 4.1.
I created a new TestRepo on our server, added a single test commit to it, added the repo to RB and confirmed the same behaviour.
Going to the path you showed with a browser results in:

{
"node": "cfc8f2c7526721829db8793e2638f4e4544930d8",
"query": "branch(.)",
"mode": "revset expression search",
"changesets": [{
"node": "cfc8f2c7526721829db8793e2638f4e4544930d8",
"date": [1487809516.0, -46800],
"desc": "A test file",
"user": "Sebastian Unger \u003csebastian.unger@taitradio.com\u003e"
}]
}

chipx86
#4 chipx86

Thanks!

The new code here isn't properly falling back in the case where an older version of Mercurial is installed. We'll need to update things to either turn off support for Mercurial < 3.9 or, ideally, aim to support older versions.

Looks like the contribution we have only supported 3.9+ because it needed data for the parent commit for each commit returned in the log, which older versions don't support. However, we work around this for other hosting services by defaulting to using the parent for the previous/next commit in the list (depending on results ordering) and reducing the number of results by 1, which we can probably do here. Also, they seem to have renamed changesets to entries in 3.9.

I'll see if someone can work on a patch for this for the next release.

  • -NeedInfo
    +Confirmed
brennie
#5 brennie
  • +brennie
seb
#6 seb

Let me know if you need any further info.

Misery
#7 Misery

Is it possible to switch the hgweb path in your repository configuration to a local file path?
Mercurial 3.9 is required for hgweb only on the "repository hosting"-server. How do you serve your hg repositories?

See:
https://bz.mercurial-scm.org/show_bug.cgi?id=5074
https://www.mercurial-scm.org/repo/hg/log?rev=json%3A

Could you paste your used URL of the json output?

seb
#8 seb

Interestingly, we already have locally modified stylesheets. In particular I had already patched the json style to implement the search API for a different use case. So it was fairly straight-forward to take the JSON template from https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/templates/json/map and drop it into place in our server. I did have to patch it a little since the utf8 filter isn't available in 3.7.

I can confirm that json search works (now looking slightly different from my implementation, but that's ok) using a browser. The output of the log url above however has not changed. The full URL btw was:
http://hg.terminals.taitradio.net:8005/TTDE/TestRepo/json-log/?rev=branch(.) which of course is not accessible from outside our intranet.

However, none of this has changed the behaviour of RB. I suspect because, looking at the original RB back-trace, RB is accessing HG using something like the RL parameter ?api_format=json which as far as I can tell has no effect on my mercurial 3.7. What's the difference between api_format=json and style=json? Because the latter does work.

Upgrading the mercurial server would be a much bigger job then patching the templates.

Misery
#9 Misery

Thanks for your information!

I can reproduce your stacktrace here with a local virtualenv with Mercurial 3.7.3.
But if I try "json-log/?rev=branch(.)" directly in the browser I get a "not yet implemented" as it is not Mercurial 3.9 or above. I cannot see how do you get the JSON structure from above. Is this your custom implementation of json style!?
Do you have your changes to Mercurial as patch?

The URL with "api_format=json" is a ReviewBoard call. This isn't the call to hgweb... if you enable the log level "INFO" on reviewboard you can see the real hgweb call like this:

2017-02-24 08:19:11,406 - DEBUG - - Initialized HgWebClient with url=u'http://localhost:8000/', username=u''
2017-02-24 08:19:11,406 - INFO - - Fetching file from http://localhost:8000/json-log/?rev=ancestors(cef139d5adc0fd512f8e62dd802a174c749ec1cc)+and+branch(default)

Misery
#10 Misery

Quickfix: https://reviews.reviewboard.org/r/8778/

seb
#11 seb

Re json style log/search: As mentioned above, I just replaced the json style map with the one downloaded from https://www.mercurial-scm.org/repo/hg/file/96eaefd350ae/mercurial/templates/json and removed all occurrences of the string 'utf8|'.

Re apt_format: I see. Using a similar URL to the one from the INFO line in your log I get:
GET http://hg.terminals.taitradio.net:8005/TTDE/TestRepo/json-log/?rev=ancestors(default)+and+branch(default)

{
"node": "8175bca2074b152dcf2d2e391aa22617857ea38f",
"query": "ancestors(default) and branch(default)",
"entries": [{
"node": "8175bca2074b152dcf2d2e391aa22617857ea38f",
"date": [1487926384.0, -46800],
"desc": "A second test commit",
"branch": "default",
"bookmarks": [],
"tags": ["tip"],
"user": "Sebastian Unger \u003csebunger44@gmail.com\u003e",
"phase": "public",
"parents": ["cfc8f2c7526721829db8793e2638f4e4544930d8"]
}, {
"node": "cfc8f2c7526721829db8793e2638f4e4544930d8",
"date": [1487809516.0, -46800],
"desc": "A test file",
"branch": "default",
"bookmarks": [],
"tags": [],
"user": "Sebastian Unger \u003csebastian.unger@taitradio.com\u003e",
"phase": "public",
"parents": []
}]
}

I assume the quick-fix will just prevent the error, right? If we can get this json-style to work, will the quick-fix allow the use of the new post-from-commit functionality with 3.7? Because it is really useful.

seb
#12 seb

I left a few comments on the review you posted. Feel free to ignore them if I don't make sense. Since I've never looked at the RB code before it is likely due to me not knowing what I'm talking about.

Anyhow, I'm off to bed. Have a good weekend and thanks for looking into this.

Misery
#13 Misery

Ok, I updated the "quickfix".
I patched a 3.7.3 with /templates/json from current stable branch of Mercurial and removed |utf8 and it works here.
So yeah, the quickfix will recognize if you patch an old version.

By the way... maybe you want to try my "pre-commit" hook of https://reviews.reviewboard.org/r/8554/ instead of a "post-commit". :-)

Misery
#14 Misery

is it fixed for you? :-)

seb
#15 seb

Our RB is still at 2.5.9. Has a new version been released?

seb
#16 seb

Ok, I'm really confused now. I just had another look at our (2.5.9) RB instance and it is now showing the create-from-commit part of the create new review request but only on some (ok, most) of our mercurial repositories. One (in particular the TestRepo I created so I can experiment with this) is still generating the "internal error" message and email. This begs two questions:

  • What's different about the TestRepo from all the other ones? (The only thing I can come up with is: It only has two commits in it, whereas all others have 1000's)
  • I thought the quick-fix above was needed to achieve this? If so, how the hack does it work with 2.5.9? Or is the quickfix only needed to not generate an exception when running with an old and unpatched mercurial server, but since I have patched the HG server the stock 2.5.9 now works?

Can anybody cure some of my confusion?

chipx86
#17 chipx86

Having the result of that request from both working and non-working servers will help. I don't know off-hand why it'd only be working on some... What version of hgweb is being used on each?

Misery
#18 Misery

Well, I don't know your infrastructure. :-)

  • If you patched Mercurial... it will work
  • If you updated Mercurial to 3.9 or higher.... it will work
  • If you use Mercurial lower than 3.9 AND use local repositories (no hgweb) ... it will work
  • If you use Mercurial lower than 3.9 and use my fix... it won't work. But do not show any "error".

By the way... "Mercurial" means "Repository-Hosting-Server-Mercurial".

david
#19 david

Fixed in release-2.5.x (8eba94c). This will ship in 2.5.10. Thanks!

  • -Confirmed
    +Fixed