1088823 - Errors when publishing commits on the Commits tab are silent

Status: RESOLVED WORKSFORME

(MozReview Graveyard :: General, defect, P1)

Description

[Gregory Szorc [:gps]]

If we encounter an error when publishing commits via the commits tab, we don't get any visual feedback.

For example, push a commit referencing a bug that does not exist and attempt to publish that review request.

This is a P1 because it is a horrible user experience and potentially leaves the system in an inconsistent state.

Comment 1

[Gregory Szorc [:gps]]

On first glance, this is a bug in Review Board.

We are making a PUT to http://localhost:50435/api/review-requests/1/draft/. The response is a 500 with .stat and .err properties. However, Review Board is looking for a .fields in reviewRequestEditorModel.js:142, not seeing one, and an Error is raised by JS.

There was a recent change to Review Board (https://github.com/reviewboard/reviewboard/commit/6e9a878100d8de960c3626e3780c1ea10057b33e) that improved error handling in this file. I'm not sure if there are still more bugs or whether we're not handling the error properly in our custom JS.

Comment 2

[Gregory Szorc [:gps]]

Tracing through it some more, I'm *very* convinced this is a Review Board bug.

We're calling into RB.ReviewRequestEditor like so:

          editor.setDraftField('targetPeople', reviewers, {
            jsonFieldName: 'target_people',
              success: function() {
                editor.setDraftField('public', true, {
                  jsonFieldName: 'public',
                  success: function() {
                    console.log('Successfully published - reloading page...');
                    $("#publish").prop("disabled", false);
                    location.reload();
                  },
                  error: function(aMsg) {
                    $("#publish").prop("disabled", false);
                    reportError(aMsg);
                  }
                })
              },
              error: function(aErrorObject) {
                reportError('Failed to set reviewers on squashed review: '
                            + aErrorObject.errorText);
              }


This in turns calls into the following code, which raises an Error during the error handling path that is supposed to call our error handler:

          reviewRequest.draft.save({
            data: data,
            error: function(model, xhr) {
                var rsp,
                    fieldValue,
                    fieldValueLen,
                    message = '';

                this.set('publishing', false);

                if (_.isFunction(options.error)) {
                    rsp = xhr.errorPayload;
                    fieldValue = rsp.fields[jsonFieldName];
                                    ^ Error here (.fields doesn't exist)

Comment 3

[Gregory Szorc [:gps]]

Filed upstream: https://code.google.com/p/reviewboard/issues/detail?id=3637

Comment 4

[Steven MacLeod [:smacleod]]

It looks like the linked Review Board issue was attempted to be reproduced on 2.0.12 and couldn't be. gps are you still seeing this?

Comment 5

[Gregory Szorc [:gps]]

I'm not even sure how I managed to do this. So I can't reproduce :/

Comment 6

[Mark Côté [:mcote]]

Reopen if you ever see this again.