[GitHub] [arrow] thisisnic commented on a diff in pull request #12839: ARROW-16154: [R] Errors which pass through `handle_csv_read_error()` and `handle_parquet_io_error()` need better error tracing

[GitBox <gi...@apache.org>]

thisisnic commented on code in PR #12839:
URL: https://github.com/apache/arrow/pull/12839#discussion_r847057232


##########
r/R/csv.R:
##########
@@ -200,8 +200,8 @@ read_delim_arrow <- function(file,
 
   tryCatch(
     tab <- reader$Read(),
-    error = function(e) {
-      handle_csv_read_error(e, schema)
+    error = function(e, call = caller_env(n = 4)) {

Review Comment:
   > Is it always n = 4?
   It's always `n = 4` here, though I deliberately chose to pass the `call` parameter into `handle_csv_read_error()` so the function could be used elsewhere in the code where we may want to pass in a different environment.  
   
   > Is there a more certain way to capture this? (Like, if you define call_env outside of tryCatch, is it just this env?)
   I could call `rlang::current_env()` above the `tryCatch` block - I went for calling `caller_env()` here as it felt "cleaner" to keep that code within this block here.
   
   I suppose that if the `tryCatch` block was changed to have more functions wrapped round it, then the number would be wrong; however, if we call `current_env()` outside of the block, we're unnecessarily calling it every time we call the function, even if there's no error.
   
   Not sure what's better - what do you think?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org