Don't cause a 500 error in the LMS if get_block unexpectedly returns None

[bradenmacdonald]

Description: We ran into a weird issue in a live course, where get_block returned None in the following code:

for child_id in self.children:
    child = self.runtime.get_block(child_id)

Since we didn't expect child ever to be None with that API, this resulted in AttributeError: 'NoneType' object has no attribute 'render', causing the whole subsection of the LMS to be replaced with a 500 error.

Although get_block shouldn't return None if everything is working, it is an expected return value when an error occurs, so we should be able to handle it gracefully.

I added an error message to the rendered HTML so that there will at least be some indication that content is missing - otherwise this could result in content silently disappearing for students even though instructors can see it fine :-/

Merge deadline: ASAP, targeting this week's release

Reviewers: @smarnach

attn: @antoviaque

[smarnach]

Nit: If this fails, the new version gives a rather unspecific error message ("expected True, but found False", or something like that). I suggest adding a message like "expected link ending in {link_href} but found {href}" (though we could also do that if the test ever fails, which might be never).

[smarnach]

👍 The code and the test look good, and I only had minor comments on the unrelated test fix.

I can't really test this fix, since I can't reproduce the original error locally (at least not with reasonable effort), but the code change looks safe, and the test covers the newly added code path.

[bradenmacdonald]

Thanks @smarnach. I amended the second commit to address your comments.

[bradenmacdonald]

@cpennington Could you please review this? It's a little fix to Problem Builder to avoid the 500 error we've been looking at in https://openedx.atlassian.net/browse/PLAT-772 .

[cpennington]

If it's a permission issue, then is it correct to call it an error?

[bradenmacdonald]

Good point, but since there's no mechanism for setting permissions on child blocks at the moment, I think it would indicate an error, at least with the current runtime.

BTW, does the API explicitly state whether the "children" property may include blocks that the user doesn't have permission to access?

[bradenmacdonald]

Also, in this case, if permissions are the problem, I would consider it a "configuration error" - the block is not designed to handle variable children, so if an author has configured it that way, they're using it incorrectly. Thankfully though that shouldn't be an issue in theory since there is no mechanism to do that AFAIK.

[cpennington]

👍