Unusual feedback path is causing an assertion failure when downloading results

[damithc]

v8.16, production

A user reported that when she tries to download results of a session, TEAMMATES reports a server error.

I tried downloading one question at a time, and noticed that the following question is the one causing the error:

Note the feedback path is probably not what the instructor intended (and there will not be any responses for that question, unless the instructor submitted something herself) but it is still a legit path.

Error in logs:

java.lang.AssertionError at teammates.logic.core.FeedbackQuestionsLogic.getRecipientsOfQuestion(FeedbackQuestionsLogic.java:278) at teammates.logic.core.FeedbackQuestionsLogic.buildCompleteGiverRecipientMap(FeedbackQuestionsLogic.java:462) at teammates.logic.core.FeedbackResponsesLogic.buildMissingResponses(FeedbackResponsesLogic.java:546) at teammates.logic.core.FeedbackResponsesLogic.buildResultsBundle(FeedbackResponsesLogic.java:432) at teammates.logic.core.FeedbackResponsesLogic.getSessionResultsForCourse(FeedbackResponsesLogic.java:478) at teammates.logic.api.Logic.getSessionResultsForCourse(Logic.java:1262) at teammates.ui.webapi.GetSessionResultsAction.execute(GetSessionResultsAction.java:76) at teammates.ui.webapi.GetSessionResultsAction.execute(GetSessionResultsAction.java:15) at teammates.ui.servlets.WebApiServlet.invokeServlet(WebApiServlet.java:65) at teammates.ui.servlets.WebApiServlet.doGet(WebApiServlet.java:38) at javax.servlet.http.HttpServlet.service(HttpServlet.java:687) at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) at ...

[ErnestCuong]

Hi prof, can I ask if someone has already worked on this issue? May I try investigating the issue?

[damithc]

Hi prof, can I ask if someone has already worked on this issue? May I try investigating the issue?

@ErnestCuong Unlikely anyone else is working on it, as no one has commented in this thread yet. So, go ahead.

[ErnestCuong]

Hi prof @damithc, I am unable to replicate the problem as you have mentioned. This is the question that I tried to edit into what was shown in your picture.

I am on the current version (after commit #11812) of the master branch. This session is created using point-based option. I downloaded the results using via the instructor home screen. I was able to download the results just fine.

[ErnestCuong]

I will continue investigating the error logs

[damithc]

I will continue investigating the error logs

Yes @ErnestCuong , there may be an other compounding factors (in addition to the odd feedback path) that combines to cause the assertion failure. Looking into the assertion code might yield some clues as well i.e., what exactly is the assertion condition that evaluates to False and how can it be False?

[ErnestCuong]

I might have found the issue. It is in the file FeedbackQuestionsLogic

In getRecipientsOfQuestion, there is assert instructorGiver != null || studentGiver != null; on line 278

In buildCompleteGiverRecipientMap, getRecipientsOfQuestion is used with either instructorGiver or studentGiver being null, hence causing the assertion error.

buildCompleteGiverRecipientMap is only used when the recipientType is not specified, as shown in the feedback path, which is why the error happened.

I am trying to replicate this error on my end. I suspect that unless there is at least one response submitted, these methods might not be used hence no error showed up when I tried downloading.

Edit: My mistake. The assertion uses the || operator so it shouldn't cause any problem.

[ErnestCuong]

Hi prof @damithc , may I ask how to download the questions individually?

[damithc]

Hi prof @damithc , may I ask how to download the questions individually?

Click on the question number

[ErnestCuong]

Hi prof, I notice that the path shown in the error log might not be used by the download function anymore. It uses the code in the typescript file now rather than the java file. I will continue my investigation to see if this is true.

[ErnestCuong]

Hi prof, so I took a crash course on servlets and I think my previous assessment was wrong. I am looking at the potential cause for both studentGiver and instructorGiver to be null, in this case specifically instructorGiver since the path is about the instructor giving feedback on nobody specific.

[damithc]

Hi prof, so I took a crash course on servlets and I think my previous assessment was wrong. I am looking at the potential cause for both studentGiver and instructorGiver to be null, in this case specifically instructorGiver since the path is about the instructor giving feedback on nobody specific.

Keep digging and let us know if you find something promising.

[ErnestCuong]

Hi prof @damithc , I am able to reproduce the problem. Apparently, this will occur when there are 2 or more instructors, with the session creator giving the feedback(? may not need) and then quitting the course, and the other instructors download the session results.

[damithc]

Hi prof @damithc , I am able to reproduce the problem. Apparently, this will occur when there are 2 or more instructors, with the session creator giving the feedback(? may not need) and then quitting the course, and the other instructors download the session results.

Good work @ErnestCuong Once you've pinned down the cause, you can proceed to attempt a fix (if you like).

[ErnestCuong]

Hi prof, I would like to attempt a fix.

[damithc]

Hi prof, I would like to attempt a fix.

Sure @ErnestCuong go ahead.

[ErnestCuong]

Hi prof, I have been able to narrow down to where a change can be made. Apparently, every session keeps track of its creator's email, but the issue arises when that email is used to find the creator (attributes) in the course from which they have already quitted, hence causing the search to return null and thus the assertion error. I am thinking about a few possible solutions:

1. Adding a field in each session that contains the creator's attributes that will not be deleted upon the creator's exit from the course (eliminating the need for a search).
2. Changing the search by email so that it will return an object that acts as a deleted user instead of a null.
3. Removing the comment from the creator entirely.

Method 1 will allow us to retain all the information about who created the session. For method 2, the creator's name will not be found, only the email remains (and possibly the message but I have not checked this yet)."

Edit: Please feel free to suggest any new solution that you think might be better :D

[damithc]

Thanks for the update @ErnestCuong You'll need the dev team's advice on which alternative to take. Given this is a corner case, adding a new field into the database may be too drastic though.