covr icon indicating copy to clipboard operation
covr copied to clipboard

Corrupt rds files?

Open hadley opened this issue 3 years ago • 4 comments
trafficstars

Over in https://github.com/duckdb/duckdb/pull/3718, covr is failing with:

Error in `readRDS()`:
! error reading from connection
Backtrace:
     ▆
  1. └─devtools:::test_coverage()
  2.   └─covr::package_coverage(pkg$path, ...)
  3.     ├─covr:::merge_coverage(trace_files) at covr/R/covr.R:494:2
  4.     └─covr:::merge_coverage.character(trace_files) at covr/R/covr.R:575:2
  5.       └─base::lapply(...) at covr/R/covr.R:580:2
  6.         └─covr (local) FUN(X[[i]], ...)
  7.           ├─base::as.list(suppressWarnings(readRDS(f))) at covr/R/covr.R:581:4
  8.           ├─base::suppressWarnings(readRDS(f))
  9.           │ └─base::withCallingHandlers(...)
 10.           └─base::readRDS(f)

The rds file exists and has data in it, but it seems to be corrupt somehow. Any ideas what's going on or how to explore further? e.g. I'd like to understand what save_trace() is seeing, but that's hard to debug since it's caller in a finalizer in another process.

hadley avatar Jun 09 '22 16:06 hadley

In this particular case accessing an ALTREP vector seems to fail, which leads to errors in saveRDS() . The error can be caught with tryCatch() .

I wonder what's a good way to propagate errors at this point to the main process. Should we tryCatch() and save the exception object in this case, which then could be picked up by merge_coverage() ?

krlmlr avatar Jul 31 '22 18:07 krlmlr

I think most likely what is happening is the main R process exists before the RDS is fully written by the child. https://github.com/r-lib/covr/issues/445 had a similar issue.

jimhester avatar Aug 01 '22 13:08 jimhester

Not sure what we can really do to recover gracefully if the trace files are corrupted, the coverage is going to be incomplete in that case. Reporting incomplete coverage could be worse than having the process fail, as it could leave you to believe parts do not have coverage when they actually do.

jimhester avatar Aug 01 '22 13:08 jimhester

I'm positive that the exit is due to a traceable error -- I could tryCatch() the saveRDS() call and enter the error = handler. One question that remains is what to do in case of an error here.

krlmlr avatar Aug 01 '22 14:08 krlmlr