[CD] Question submission server error

[sagangwee]

"Found extra args" error:

Contributor.Dashboard.-.Oppia.2.mp4

[sagangwee]

Looks like an image path is being provided to SuggestionHandler that is not expected.

[sagangwee]

Reproduced in prod.

For a payload like the following:

{
   "suggestion_type":"add_question",
   "target_type":"skill",
   "description":"Add new question",
   "target_id":"G0NsFwEahIZx",
   "target_version_at_submission":8,
   "change":{
      "cmd":"create_new_fully_specified_question",
      "question_dict":{
         "id":null,
         "question_state_data":{
            "content":{
               "html":"<p>TESTING DO NOT SUBMIT</p>\n\n<p>Multiply&nbsp;<oppia-noninteractive-math math_content-with-value=\"{&amp;quot;raw_latex&amp;quot;:&amp;quot;10^3&amp;quot;,&amp;quot;svg_filename&amp;quot;:&amp;quot;mathImg_20220309_141302_k1lqmfdd4l_height_2d49_width_3d407_vertical_0d241.svg&amp;quot;}\" ng-version=\"11.2.14\"></oppia-noninteractive-math></p>",
               "content_id":"content"
            },
            "classifier_model_id":null,
            "linked_skill_id":null,
            "interaction":{
               "answer_groups":[
                  {
                     "rule_specs":[
                        {
                           "rule_type":"FuzzyEquals",
                           "inputs":{
                              "x":{
                                 "contentId":"rule_input_2",
                                 "normalizedStrSet":[
                                    "t"
                                 ]
                              }
                           }
                        }
                     ],
                     "outcome":{
                        "dest":null,
                        "feedback":{
                           "html":"<p>t</p>",
                           "content_id":"feedback_1"
                        },
                        "labelled_as_correct":true,
                        "param_changes":[
                           
                        ],
                        "refresher_exploration_id":null,
                        "missing_prerequisite_skill_id":null
                     },
                     "training_data":[
                        
                     ],
                     "tagged_skill_misconception_id":null
                  },
                  {
                     "rule_specs":[
                        {
                           "rule_type":"FuzzyEquals",
                           "inputs":{
                              "x":{
                                 "contentId":"rule_input_6",
                                 "normalizedStrSet":[
                                    "f"
                                 ]
                              }
                           }
                        }
                     ],
                     "outcome":{
                        "dest":null,
                        "feedback":{
                           "html":"<p>No, that's not correct. Remember, when multiplying by the tens, hundreds, etc. digit of the second factor, you should write your answer so that its last digit lies in the tens, hundreds, etc. place. Instead, you've written all the intermediate products so that they end at the units place. Try again!</p><p>(In the future, you can catch mistakes like this for yourself by <oppia-noninteractive-skillreview skill_id-with-value=\"&amp;quot;OQUsLvCg6dAX&amp;quot;\" text-with-value=\"&amp;quot;estimation&amp;quot;\"></oppia-noninteractive-skillreview>.)</p>",
                           "content_id":"feedback_5"
                        },
                        "labelled_as_correct":false,
                        "param_changes":[
                           
                        ],
                        "refresher_exploration_id":null,
                        "missing_prerequisite_skill_id":null
                     },
                     "training_data":[
                        
                     ],
                     "tagged_skill_misconception_id":"G0NsFwEahIZx-0"
                  },
                  {
                     "rule_specs":[
                        {
                           "rule_type":"FuzzyEquals",
                           "inputs":{
                              "x":{
                                 "contentId":"rule_input_8",
                                 "normalizedStrSet":[
                                    "r"
                                 ]
                              }
                           }
                        }
                     ],
                     "outcome":{
                        "dest":null,
                        "feedback":{
                           "html":"<p>No, that's not correct. You are treating each of the digits in the factors individually, but it is important to consider the place value as well. For example, in 24 * 35, you would calculate (20 + 4) * (30 + 5), not (2 + 4) * (3 + 5).</p>\n\n<p>If you've forgotten how to do multi-digit multiplication, check this <oppia-noninteractive-skillreview _nghost-lke-c11=\"\" ng-version=\"11.2.14\" skill_id-with-value=\"&amp;quot;G0NsFwEahIZx&amp;quot;\" text-with-value=\"&amp;quot;concept card&amp;quot;\"></oppia-noninteractive-skillreview> for a refresher.</p>\n\n<p>Try again!</p>",
                           "content_id":"feedback_7"
                        },
                        "labelled_as_correct":false,
                        "param_changes":[
                           
                        ],
                        "refresher_exploration_id":null,
                        "missing_prerequisite_skill_id":null
                     },
                     "training_data":[
                        
                     ],
                     "tagged_skill_misconception_id":"G0NsFwEahIZx-1"
                  },
                  {
                     "rule_specs":[
                        {
                           "rule_type":"FuzzyEquals",
                           "inputs":{
                              "x":{
                                 "contentId":"rule_input_10",
                                 "normalizedStrSet":[
                                    "f"
                                 ]
                              }
                           }
                        }
                     ],
                     "outcome":{
                        "dest":null,
                        "feedback":{
                           "html":"<p>No, that's not correct. That's not how multi-digit multiplication works.</p>\n\n<p>If, for example, you were calculating 35 * 61, then you would need to multiply (30 + 5) and (60 + 1). You wouldn't just multiply 3 * 6 and 5 * 1, because this ignores the fact that the 3 and 6 have a different place value (they are in the tens place, and should be treated as 30 and 60 instead). In addition, the cross-terms of \"30 * 1\" and \"5 * 60\" would be missing.</p>\n\n<p>Take a look at this <oppia-noninteractive-skillreview _nghost-lke-c11=\"\" ng-version=\"11.2.14\" skill_id-with-value=\"&amp;quot;G0NsFwEahIZx&amp;quot;\" text-with-value=\"&amp;quot;concept card&amp;quot;\"></oppia-noninteractive-skillreview> on multi-digit multiplication to understand the steps, and then try the question again.</p>",
                           "content_id":"feedback_9"
                        },
                        "labelled_as_correct":false,
                        "param_changes":[
                           
                        ],
                        "refresher_exploration_id":null,
                        "missing_prerequisite_skill_id":null
                     },
                     "training_data":[
                        
                     ],
                     "tagged_skill_misconception_id":"G0NsFwEahIZx-4"
                  },
                  {
                     "rule_specs":[
                        {
                           "rule_type":"FuzzyEquals",
                           "inputs":{
                              "x":{
                                 "contentId":"rule_input_12",
                                 "normalizedStrSet":[
                                    "f"
                                 ]
                              }
                           }
                        }
                     ],
                     "outcome":{
                        "dest":null,
                        "feedback":{
                           "html":"<p>No, that's not correct. The question is asking you to <strong>multiply </strong>the two numbers, not add them!</p>\n\n<p>Try again.</p>",
                           "content_id":"feedback_11"
                        },
                        "labelled_as_correct":false,
                        "param_changes":[
                           
                        ],
                        "refresher_exploration_id":null,
                        "missing_prerequisite_skill_id":null
                     },
                     "training_data":[
                        
                     ],
                     "tagged_skill_misconception_id":"G0NsFwEahIZx-6"
                  }
               ],
               "confirmed_unclassified_answers":[
                  
               ],
               "customization_args":{
                  "rows":{
                     "value":1
                  },
                  "placeholder":{
                     "value":{
                        "unicode_str":"t",
                        "content_id":"ca_placeholder_0"
                     }
                  }
               },
               "default_outcome":{
                  "dest":null,
                  "feedback":{
                     "html":"<p>t</p>",
                     "content_id":"default_outcome"
                  },
                  "labelled_as_correct":false,
                  "param_changes":[
                     
                  ],
                  "refresher_exploration_id":null,
                  "missing_prerequisite_skill_id":null
               },
               "hints":[
                  {
                     "hint_content":{
                        "html":"<p>t</p>",
                        "content_id":"hint_3"
                     }
                  }
               ],
               "id":"TextInput",
               "solution":{
                  "answer_is_exclusive":false,
                  "correct_answer":"t",
                  "explanation":{
                     "html":"<p>t</p>",
                     "content_id":"solution"
                  }
               }
            },
            "param_changes":[
               
            ],
            "recorded_voiceovers":{
               "voiceovers_mapping":{
                  "content":{
                     
                  },
                  "default_outcome":{
                     
                  },
                  "ca_placeholder_0":{
                     
                  },
                  "feedback_1":{
                     
                  },
                  "rule_input_2":{
                     
                  },
                  "hint_3":{
                     
                  },
                  "solution":{
                     
                  },
                  "feedback_5":{
                     
                  },
                  "rule_input_6":{
                     
                  },
                  "feedback_7":{
                     
                  },
                  "rule_input_8":{
                     
                  },
                  "feedback_9":{
                     
                  },
                  "rule_input_10":{
                     
                  },
                  "feedback_11":{
                     
                  },
                  "rule_input_12":{
                     
                  }
               }
            },
            "solicit_answer_details":false,
            "card_is_checkpoint":false,
            "written_translations":{
               "translations_mapping":{
                  "content":{
                     
                  },
                  "default_outcome":{
                     
                  },
                  "ca_placeholder_0":{
                     
                  },
                  "feedback_1":{
                     
                  },
                  "rule_input_2":{
                     
                  },
                  "hint_3":{
                     
                  },
                  "solution":{
                     
                  },
                  "feedback_5":{
                     
                  },
                  "rule_input_6":{
                     
                  },
                  "feedback_7":{
                     
                  },
                  "rule_input_8":{
                     
                  },
                  "feedback_9":{
                     
                  },
                  "rule_input_10":{
                     
                  },
                  "feedback_11":{
                     
                  },
                  "rule_input_12":{
                     
                  }
               }
            },
            "next_content_id_index":13
         },
         "question_state_data_schema_version":1,
         "language_code":"en",
         "linked_skill_ids":[
            "G0NsFwEahIZx"
         ],
         "inapplicable_skill_misconception_ids":[
            "G0NsFwEahIZx-2",
            "G0NsFwEahIZx-3",
            "G0NsFwEahIZx-5"
         ],
         "version":0
      },
      "skill_id":"G0NsFwEahIZx",
      "skill_difficulty":0.3
   }
}

Got a response:

{
   "error":"Found extra args: ['img_20200712_154524_88sfjivzr9_height_166_width_132.svg', 'img_20200711_165515_oqmu90mz19_height_71_width_132.svg', 'img_20200712_154812_m0biksf8pz_height_211_width_140.svg', 'img_20200711_170526_85sbwj9x81_height_211_width_140.svg', 'img_20200711_165602_yjboiwlroj_height_116_width_132.svg', 'img_20200712_154504_vys35ybfbl_height_148_width_132.svg'].",
   "status_code":400
}

[sagangwee]

Looks like we pass in image blobs retrieved from storage to the suggestion handler here: https://github.com/oppia/oppia/blob/develop/core/templates/pages/contributor-dashboard-page/modal-templates/question-suggestion-editor-modal.controller.ts#L98, but the handler expects a "files" arg: https://github.com/oppia/oppia/blob/develop/core/controllers/suggestion.py#L83.

[sagangwee]

https://github.com/oppia/oppia/pull/14063/files migrated the schema to use the "files" handler arg, but calls to question suggestions were not updated.

[kevintab95]

Hi @sagangwee, is the payload mentioned in #15101 (comment) the complete thing? I'm unable to find the filenames ['img_20200712_154524_88sfjivzr9_height_166_width_132.svg', 'img_20200711_165515_oqmu90mz19_height_71_width_132.svg', 'img_20200712_154812_m0biksf8pz_height_211_width_140.svg', 'img_20200711_170526_85sbwj9x81_height_211_width_140.svg', 'img_20200711_165602_yjboiwlroj_height_116_width_132.svg', 'img_20200712_154504_vys35ybfbl_height_148_width_132.svg']." in the payload.

[sagangwee]

@kevintab95 Sorry, I do recall the img file names being listed below the payload in Chrome Dev Tools. I didn't log them on my first pass as I didn't realize they were part of the request at the time.

[kevintab95]

Thanks! One more follow-up, looking at the question dict, I can't find any oppia-noninteractive-image tags with the corresponding filenames as attributes. Which is odd.. where are these image filenames coming from?

[kevintab95]

Also, in the video in the issue description, it seems like there are no images (except for the math one) in the submitted question. Are the logs and video from the same session / set of operations?

[sagangwee]

The images are loaded from ImageLocalStorageService, but I'm not familiar with it.

I think the fix is to add the image blobs to a "files" arg here: https://github.com/oppia/oppia/blob/develop/core/templates/pages/contributor-dashboard-page/services/question-suggestion-backend-api.service.ts#L60 like what was done for translations: https://github.com/oppia/oppia/pull/14063/files#diff-6e706463b3e6fb489f0eab117dabc7ec24080746f2ef815593b9c608094ef09eR101.

[kevintab95]

Update: This error happens when trying to submit a question (with or without images) for a skill that contains images.

[seanlip]

Thanks for investigating, @kevintab95!

Just to check, what do you mean by "a skill that contains images"? Which field of the skill are you referring to?

(Also that is weird. I don't really see why we would want to submit those images back to the server...)

[kevintab95]

Skills can contain images in their subtitledHTML fields (e.g. description). I suspect these may be used in the preview or the question player,not sure.

[kevintab95]

Misconceptions can contain images as well (the "default feedback" in the misconception can be used in the questions).

[seanlip]

But why do we send these back to the server? That seems odd, the CD should only be submitting the question + the skill ID.

(Though I guess that's what you're investigating :P )

[kevintab95]

From what I understand, images are stored in model specific buckets, so all images related to the question entity will need to be stored in the question specific bucket. Some images seem to be in skill specific buckets (e.g. the description, misconceptions) so we copy them over to the question specific one -- I imagine this is why we send it to the server.

[seanlip]

Ah I see. Just to check ... I can see this happening for misconceptions that are ultimately submitted as part of the question, but we don't do that for the skill description, right? (At least, conceptually, I think we shouldn't, since the description is never part of the question.)

[kevintab95]

True, currently I think we look at all HTML fields in the skill, we can restrict it to specific ones.