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

Capture error in eng_sql if error = TRUE #1910

Merged
merged 3 commits into from
Oct 22, 2020
Merged

Capture error in eng_sql if error = TRUE #1910

merged 3 commits into from
Oct 22, 2020

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Oct 22, 2020

This will fix rstudio/rmarkdown#1208

We capture the error of SQL execution if error = TRUE as for some other engine.

This allows this Rmd to work now without stoping

---
title: "Error Honored"
output: html_document
---

```{r, message=FALSE}
knitr::opts_chunk$set(error = TRUE)
library(DBI)
con <- dbConnect(RSQLite::SQLite(), ":memory:")
```

```{r}
stop('error message that I want to print')
```

```{r}
dbWriteTable(con, "mtcars", mtcars)
```

```{sql, connection=con}
SELECT * FROM mtcars WHERE cyl = 4
```

```{sql, connection=con}
DROP TABLE mtcars
```

```{r}
dbListTables(con)
```

```{sql, connection=con}
select test;
```

I'll do a PR in knitr-example to complement this PR with tests

Copy link
Owner

@yihui yihui left a comment

Choose a reason for hiding this comment

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

Thanks!

@yihui yihui merged commit f409cd9 into master Oct 22, 2020
@yihui yihui deleted the error-eng-sql branch October 22, 2020 17:45
@cderv
Copy link
Collaborator Author

cderv commented Oct 22, 2020

One thought after having pushed that...

How does that work in knitr so that the error hook is used ? For example if someone want to use a specific class.error, I am not sure what I have done is enough. More generally, I don't think for non R engine that wrap.error will be called correctly.
Should this be improve ?

@yihui
Copy link
Owner

yihui commented Oct 22, 2020

This could be improved. The out argument of engine_output() can take a list. You could add the error message to the list, and make its class to be error, then knitr:::wrap should handle it.

The work is probably not trivial, so I'd rather wait for someone to request it before we spend the time on the improvement.

@cderv
Copy link
Collaborator Author

cderv commented Oct 22, 2020

Thanks.

Let's wait then.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RNotebooks - Add error=TRUE support for sql chunks
2 participants