4988: Fail to post diff with perforce repo: 'Value to convert is unexpected type %s', <class 'rev iewboard.scmtools.core.Revision'>
- Fixed
- Review Board
benjackson | |
What version are you running?
4.0.11
What's the URL of the page containing the problem?
│>>> Making HTTP POST request to http://ukfd-ltp4l:49320/api/validation/diffs/
What steps will reproduce the problem?
It doesn't seem to happen for all diffs where a new file is added, but it's certainly related to adding new files, with perforce as the repo type. Specifically, it needs to go though these lines in
perforce.py
where the old revision is set toPRE_CREATION
.# Older versions of Perforce had this lovely idiosyncracy that diffs # show revision #1 both for pre-creation and when there's an actual # revision. In this case, we need to check if the file already exists # in the repository. # # Newer versions use #0, so it's quicker to check. if (revision == b'0' or (revision == b'1' and not self.repository.get_file_exists(filename.decode('utf-8'), revision.decode('utf-8')))): revision = PRE_CREATION
- Using git-p4 client repo, add a new file and commit that
- rbt post
As i said, i debugged it a bit but wasn't 100% sure why this didn't happen for every new file add. See below for the exact exception and code which is triggering it. Attached screenshots of debugger showing the call stacks where it's assigned etc.
I can easily repro this with a specific commit right now, and have a debug environment set up, so please shout if there is more info I can provide.
What is the expected output? What do you see instead?
Diff created.
Actual: Exceptoin TypeError: ('Value to convert is unexpected type %s', <class 'reviewboard.scmtools.core.Revision'>)$>rbt post --repository perforce:1666 --server http://ukfd-ltp4l:49320 675417754ac980cc60d9b72eddaf9f9cb80817dd Unexpected error when validating the diff: ('Value to convert is unexpected type %s', <class 'reviewboard.scmtools.core.Revision'>) (API Error 224: Diff Parsing Failed) ukfd-ltp4l(benj:TESTKIT_main.git) TESTKIT/main.git>What operating system are you using? What browser?
RHEL7
Also reproduced in a minimal ubuntu 20.02 container (based on contrib/docker/Dockerfile)
Please provide any additional information below.
The issue appears to be with this code in filediff_creator.py:
# Store the information on the parent's filename and revision. # It's important we force these to text, since they may be # byte strings and the revision may be a Revision instance. # # Starting in Review Board 4.0.5, we store this any time there's # a parent diff, whether or not the file existed in the parent # diff. extra_data.update({ FileDiff._IS_PARENT_EMPTY_KEY: parent_is_empty, 'parent_source_filename': convert_to_unicode(parent_source_filename, encoding_list)[1], 'parent_source_revision': convert_to_unicode(parent_source_revision, encoding_list)[1], })Per the comment, the argument
parent_source_revision
may be aRevision
object (in this case, it is literallyPRE_CREATION
which is an instance ofRevision
, howeverconvert_to_unicode
requires arguments to be some type of string, and throws an exception when they aren'Git blame points the finger at this commit on the branch: b972f20c55 (it's the same on master)
Here's the full stack trace from the log:
2023-01-20 20:59:32,986 - ERROR - - reviewboard.webapi.resources.validate_diff - Unexpected error whe n validating diff. Traceback (most recent call last): File "/build_root/reviewboard/reviewboard/webapi/resources/validate_diff.py", line 161, in create DiffSet.objects.create_from_upload( File "/build_root/reviewboard/reviewboard/diffviewer/managers.py", line 767, in create_from_upload return self.create_from_data( File "/build_root/reviewboard/reviewboard/diffviewer/managers.py", line 1060, in create_from_data create_filediffs( File "/build_root/reviewboard/reviewboard/diffviewer/filediff_creator.py", line 210, in create_filed iffs convert_to_unicode(parent_source_revision, File "/build_root/reviewboard/reviewboard/diffviewer/diffutils.py", line 112, in convert_to_unicode raise TypeError('Value to convert is unexpected type %s', type(s)) TypeError: ('Value to convert is unexpected type %s', <class 'reviewboard.scmtools.core.Revision'>)Full output from
rbt
with--debug
:>>> RBTools 3.1.3 alpha 0 (dev) >>> Python 3.6.3 (default, Apr 10 2019, 14:37:36) [GCC 4.8.5 20150623 (Red Hat 4.8.5-16)] >>> Running on Linux-3.10.0-1160.59.1.el7.x86_64-x86_64-with-redhat-7.9-Maipo >>> Home = /mma/users/benj >>> Current directory = /mma/users/benj/proj/perforce-1666/TESTKIT/main.git >>> Making HTTP GET request to http://ukfd-ltp4l:49320/api/ >>> Unable to execute "cleartool help": skipping ClearCase >>> Running: tf vc help >>> Checking for a Bazaar repository... >>> Unable to execute "brz help" or "bzr help": skipping Bazaar >>> Checking for a VersionVault / ClearCase repository... >>> Unable to execute "cleartool help": skipping ClearCase >>> Checking for a CVS repository... >>> Checking for a Git repository... >>> Running: git rev-parse --git-dir >>> Running: git config core.bare >>> Running: git rev-parse --show-toplevel >>> Checking for a Mercurial repository... >>> Unable to execute "hg --help": skipping Mercurial >>> Checking for a Perforce repository... >>> Running: p4 info >>> Checking for a Plastic repository... >>> Unable to execute "cm version": skipping Plastic >>> Checking for a Cliosoft SOS repository... >>> Running: soscmd version >>> Unable to execute "soscmd version"; skipping SOS >>> Checking for a Subversion repository... >>> Running: svn --non-interactive info >>> Command exited with rc 1: ['svn', '--non-interactive', 'info'] ["svn: E155007: '/mma/users/benj/proj/perforce-1666/TESTKIT/main.git' is not a working copy\n"]--- >>> Checking for a Team Foundation Server repository... >>> Unable to execute "tf help": skipping TFS >>> Finding deepest repository of multiple matching repository types. >>> Running: git rev-parse --git-dir >>> Running: git config core.bare >>> Running: git rev-parse --show-toplevel >>> Running: git symbolic-ref -q HEAD >>> Running: git show-ref --verify refs/remotes/p4/master >>> Running: git config --get git-p4.port >>> Repository info: Path: perforce:1666, Base path: >>> Making HTTP GET request to http://ukfd-ltp4l:49320/api/repositories/?name=perforce%3A1666&only-fields=id%2Cname%2Cmirror_path%2Cpath&only-links=info&tool=Git%2CPerforce%2CSubversion >>> HTTP GET request to http://ukfd-ltp4l:49320/api/repositories/?name=perforce%3A1666&only-fields=id%2Cname%2Cmirror_path%2Cpath&only-links=info&tool=Git%2CPerforce%2CSubversion cannot be cached >>> Making HTTP GET request to http://ukfd-ltp4l:49320/api/repositories/1/info/ >>> Got API Error 209 (HTTP code 501): The specified repository is not able to perform this action. >>> Error data: {'err': {'code': 209, 'msg': 'The specified repository is not able to perform this action.'}, 'stat': 'fail'} >>> Command line: rbt post --debug --repository perforce:1666 --server http://ukfd-ltp4l:49320 675417754ac980cc60d9b72eddaf9f9cb80817dd >>> Running: git rev-parse 675417754ac980cc60d9b72eddaf9f9cb80817dd >>> Running: git rev-parse 675417754ac980cc60d9b72eddaf9f9cb80817dd^ >>> Running: git branch --remotes >>> Running: git rev-list f84fc277594e6098d370ddb49431abde0f11f4c7 --not --remotes=p4 >>> Running: git rev-parse 8d7c75c93cbf0b8c11bb47ca602676b8b5089165^ >>> Found youngest remote git commit b91de936426df9dbc694f37d033d213537b95de6 >>> Running: git version >>> Running: git -c core.quotepath=false diff --no-color --no-prefix -r -u --no-ext-diff f84fc277594e6098d370ddb49431abde0f11f4c7..675417754ac980cc60d9b72eddaf9f9cb80817dd >>> Running: git log b91de936426df9dbc694f37d033d213537b95de6 >>> Running: p4 files //depot/TESTKIT/main/src/tcl/testKit_Packages.tcl@2485266 >>> Running: p4 files //depot/TESTKIT/main/src/tcl/testKit_TestLibCaseSettings.tcl@2485266 >>> Running: git -c core.quotepath=false diff --no-color --no-prefix -r -u --no-ext-diff b91de936426df9dbc694f37d033d213537b95de6..f84fc277594e6098d370ddb49431abde0f11f4c7 >>> Running: git log b91de936426df9dbc694f37d033d213537b95de6 >>> Running: p4 files //depot/TESTKIT/main/src/scripts/testKit_Test@2485266 >>> Running: p4 files //depot/TESTKIT/main/src/tcl/testKit_TestLibFoundation.tcl@2485266 >>> Running: p4 files //depot/TESTKIT/main/src/tcl/testKit_TestLibReconfigure.tcl@2485266 >>> Running: p4 files //depot/TESTKIT/main/src/tcl/testKit_TestLibRunning.tcl@2485266 >>> Running: p4 files //depot/TESTKIT/main/src/tcl/testKit_TestLibSymbolExpansion.tcl@2485266 >>> Generated parent diff size: 8364 bytes >>> Making HTTP GET request to http://ukfd-ltp4l:49320/api/validation/diffs/ >>> HTTP GET request to http://ukfd-ltp4l:49320/api/validation/diffs/ cannot be cached >>> Making HTTP POST request to http://ukfd-ltp4l:49320/api/validation/diffs/ Please log in to the Review Board server at ukfd-ltp4l:49320. Username: Ben.Jackson Password: >>> Got API Error 224 (HTTP code 400): Unexpected error when validating the diff: ('Value to convert is unexpected type %s', <class 'reviewboard.scmtools.core.Revision'>) >>> Error data: {'err': {'code': 224, 'msg': "Unexpected error when validating the diff: ('Value to convert is unexpected type %s', <class 'reviewboard.scmtools.core.Revision'>)"}, 'stat': 'fail'} Traceback (most recent call last): File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 827, in make_request request.url, body, headers, request.method)) File "/mma/users/benj/proj/rbtools/rbtools/api/cache.py", line 209, in make_request return self.urlopen(request) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 223, in urlopen return opener.open(url, data, timeout) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 532, in open response = meth(req, response) File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 370, in http_response response.info()) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 564, in error result = self._call_chain(*args) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 504, in _call_chain result = func(*args) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 1028, in http_error_401 url, req, headers) File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 458, in http_error_auth_reqed self, authreq, host, req, headers) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 981, in http_error_auth_reqed return self.retry_http_basic_auth(host, req, realm) File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 546, in retry_http_basic_auth return self.parent.open(request, timeout=request.timeout) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 532, in open response = meth(req, response) File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 370, in http_response response.info()) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 570, in error return self._call_chain(*args) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 504, in _call_chain result = func(*args) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/urllib/request.py", line 650, in http_error_default raise HTTPError(req.full_url, code, msg, hdrs, fp) urllib.error.HTTPError: HTTP Error 400: Bad Request During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/mma/users/benj/proj/rbtools/rbtools/commands/post.py", line 959, in main self._validate_squashed_diff(squashed_diff) File "/mma/users/benj/proj/rbtools/rbtools/commands/post.py", line 1569, in _validate_squashed_diff **validate_kwargs) File "/mma/users/benj/proj/rbtools/rbtools/api/decorators.py", line 27, in request_method *args, **kwargs) File "/mma/users/benj/proj/rbtools/rbtools/api/transport/sync.py", line 83, in execute_request_method return self._execute_request(request) File "/mma/users/benj/proj/rbtools/rbtools/api/transport/sync.py", line 92, in _execute_request rsp = self.server.make_request(request) File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 829, in make_request self.process_error(e.code, e.read()) File "/mma/users/benj/proj/rbtools/rbtools/api/request.py", line 803, in process_error rsp['err']['msg']) rbtools.api.errors.BadRequestError: Unexpected error when validating the diff: ('Value to convert is unexpected type %s', <class 'reviewboard.scmtools.core.Revision'>) (API Error 224: Diff Parsing Failed) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/mma/users/benj/.local/bin/rbt", line 33, in <module> sys.exit(load_entry_point('RBTools', 'console_scripts', 'rbt')()) File "/mma/users/benj/proj/rbtools/rbtools/commands/main.py", line 207, in main command.run_from_argv([RB_MAIN, command_name] + args) File "/mma/users/benj/proj/rbtools/rbtools/commands/__init__.py", line 1102, in run_from_argv exit_code = self.main(*args) or 0 File "/mma/users/benj/proj/rbtools/rbtools/commands/post.py", line 970, in main % (msg_prefix, e)) rbtools.commands.CommandError: Error validating diff
Fixed in release-4.0.x (99b1e7e).
This will ship in the upcoming 5.0.2 and (at a later point), 4.0.12
Found a regression with the fix, but we'll get reworked version in for 5.0.2 (which is going out tonight/tomorrow).
force_text()
/force_str()
will assume an encoding if the object is abytes
or converts to abytes
. The problem with this is that we can't assume at this particular stage. We instead need to use the configured encoding list. This matters particularly for people working with some Chinese encodings.convert_to_unicode()
is responsible for trying the configured encodings, but since the fix was passing in the forced-encode, it never got a chance.That's not a problem for
Revision
instances, but is for raw revisions. Probably not even a problem there (revisions are generally effectively ASCII in most SCMs), but it's more of a problem when forcing the encoding for filenames (which are completely at the whim of the developers committing code).Making a change to specifically check for and cast the
Revision
before storing it instead, which should be a bit safer.