Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hand off parse error to error checker if one is available #596

Merged
merged 12 commits into from
Oct 7, 2021

Conversation

gadenbuie
Copy link
Member

  1. Errors that happen when evaluating global_setup in evaluate_exercise() are now returned as internal errors
  2. We no longer globally copy exercise.error.check.code into error_check, instead we do that operation when we actually go to evaluate the error checker. This lets us tell the difference between a global default option and an explicit error checker.
  3. The parse error checker now runs the parse error through the error checker if one is available (and it's not the default error checker).

Copy link
Contributor

@rossellhayes rossellhayes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

R/exercise.R Show resolved Hide resolved
- Internal error helper emits log messages via `message()` and an error exercise result with specificl language
- `render_exercise()` catches setup and user code errors but returns an internal error if the error comes from the setup chunk(s)
- The error condition of internal errors is returned under `$feedback$error`
- Pass the original user code error to the error checker, rather than the wrapped error we use to signal the issue
@gadenbuie
Copy link
Member Author

gadenbuie commented Oct 6, 2021

I added a few more changes that overhaul internal errors in learnr and to make sure that errors caused by exercise setup chunks aren't treated like user-generated errors:

  • Internal error helper emits log messages via message() rlang::warn() and an error exercise result with specific internal (message) or external (feedback) facing language
  • render_exercise() still catches setup and user code errors with the same tryCatch() block but returns an internal error if the error comes from the setup chunk(s)
  • The error condition of internal errors is returned under $feedback$error
  • When an error occurs and should be checked, we now pass the original user code error to the error checker, rather than the wrapped error we use to signal the issue
  • We're now more careful about when we should be running the error checks

NEWS.md Outdated Show resolved Hide resolved
@rossellhayes
Copy link
Contributor

Just to clarify f0c6366, warnings will show up in the console but not in the learnr exercise, correct?

@gadenbuie
Copy link
Member Author

gadenbuie commented Oct 7, 2021

Just to clarify f0c6366, warnings will show up in the console but not in the learnr exercise, correct?

Yup, that's emitted either in the Shiny server or in the logs of the external evaluator

@gadenbuie gadenbuie merged commit b000739 into master Oct 7, 2021
@gadenbuie gadenbuie deleted the surface-parse-errors-internally branch October 7, 2021 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants