Skip to content

Commit

Permalink
🪲 Don't crash the "view program" page if the program has a syntax err…
Browse files Browse the repository at this point in the history
…or (#5985)

It used to be that the "view program" page would crash if a program has syntax errors. The reason is that we try to translate programs from the language of the author to the language of the viewer, but that operation may fail due to parse errors, which would cause the page to fail loading.

We used to only check for blanks and avoid translating a program because we knew that programs with blanks would fail to parse; but there are many more reasons that would case a parse failure, so instead of trying to predict them I'm now just catching the error.

In this PR I want to address a failure case as quickly as possible. There are some open issues/questions that someone else could/should have a look at later:

- I have not written a test yet to make sure this doesn't re-occur.
- We do translation in 2 steps: first to English, then to the target language. I don't know if that's on purpose or not; I imagine so, it's what the original code was doing and it must be for a reason.
- The original code used to always normalize to English first, then translate to the target language. I've changed it to only translate if the source and target languages are different in order to save compute time. I hope that's doesn't break any use cases I'm not aware of.
- We have no way to surface the parse error the user. That may be fine because they can try running the program and then seeing the parse error, but just something to note.

**How to test**

Write a syntactically invalid program and save it.

Use the `/hedy/.../view_program` link to view it, and observe that the page loads successfully.

Co-Authored-By: Jesús Pelay <[email protected]>
  • Loading branch information
rix0rrr and jpelay authored Dec 2, 2024
1 parent 71c8290 commit f031876
Show file tree
Hide file tree
Showing 10 changed files with 372 additions and 44 deletions.
43 changes: 26 additions & 17 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1882,23 +1882,32 @@ def view_program(user, id):
result['username']) else None

code = result['code']
# if the snippet has blanks, it'll fail in the translation process
# so we just dont translate it if it has any
has_blanks = hedy.location_of_first_blank(code) > 0
if not has_blanks and result.get("lang") != "en" and result.get("lang") in ALL_KEYWORD_LANGUAGES.keys():
code = hedy_translation.translate_keywords(code, from_lang=result.get(
'lang'), to_lang="en", level=int(result.get('level', 1)))
# If the keyword language is non-English -> parse again to guarantee
# completely localized keywords
if not has_blanks and g.keyword_lang != "en":
code = hedy_translation.translate_keywords(
code,
from_lang="en",
to_lang=g.keyword_lang,
level=int(
result.get(
'level',
1)))
level = int(result.get('level', 1))

# Try to translate the program from the language of the program to the language of the viewer
#
# - We do this in 2 steps: first translate from the source language to English,
# then from English to the target language. There must be a reason for this, but I don't
# know what it is.
# - In the past we used to do this "normalization via English" step for every program,
# even if we would end up translating nl -> nl. I don't know if that was for a particular
# reason but I've changed it to only do the translation if the source and target languages
# are different.
# - Translation may fail, because it requires parsing the program and the program
# may be syntactically invalid (missing indentation, programming mistakes, etc). Or it
# may contain blanks, which will always yield an unparseable program.
# - We don't currently have a way to surface this error to the user. I don't know if
# that matters.
source_language = result.get("lang")
target_language = g.keyword_lang
if source_language != target_language and source_language in ALL_KEYWORD_LANGUAGES.keys():
try:
code = hedy_translation.translate_keywords(
code, from_lang=source_language, to_lang=target_language, level=level)
except Exception as e:
# Not really a good place to leave this error, but at least we don't
# want it crashing the page load. Log it as a warning then.
logger.warning(f"Error translating program {id} from {source_language} to {target_language}: {e}")

result['code'] = code
student_adventure_id = f"{result['username']}-{result['adventure_name']}-{result['level']}"
Expand Down
Loading

0 comments on commit f031876

Please sign in to comment.