ropensci / rentrez

talk with NCBI entrez using R
https://docs.ropensci.org/rentrez
Other
195 stars 38 forks source link

Consider forcing HTTP/1.1 until Entrez upgrades their Nginx install #127

Closed hammer closed 5 years ago

hammer commented 5 years ago

I am using a libcurl that attempts to use HTTP/2 by default. When using rentrez I'm getting errors on every other request, e.g.

Error in curl::curl_fetch_memory(url, handle = handle) : 
  HTTP/2 stream 3 was not closed cleanly: REFUSED_STREAM (err 7)

and

Error in curl::curl_fetch_memory(url, handle = handle) : 
  Error in the HTTP2 framing layer

When I force HTTP/1.1, using entrez_summary(..., config = httr::config(http_version = 2))1, I don't see these errors.

By using the sophisticated debugging technique of googling my error messages, I've come to believe that the source of the HTTP/2 issues is an old version of Nginx whose interaction w/ curl when using HTTP/2 is a bit janky (cf. https://github.com/curl/curl/issues/804#issuecomment-218903711).

I've emailed eutilities@ncbi.nlm.nih.gov to confirm the source of the error. While I wait to hear from them, would y'all consider forcing HTTP/1.1 on the client by default?

1 For confused readers such as future me, http_version takes values in an enum for which index 2 corresponds to HTTP/1.1

dwinter commented 5 years ago

Hi @hammer,

Thanks for this report and the suggested workaround -- very helpful. I'll try and make some time this week to see if we can set a default in a way that doesn't upset anything else rentrez is doing.

If you get a reply from the NCBI please let us know what they say and are planning.

Cheers

joelnitta commented 5 years ago

I was having the same problem, with both rentrez::entrez_fetch() and taxize::classification(). The fix mentioned above did not work for me. I was able to fix it by running httr::set_config(httr::config(http_version = 0)) before using those functions, as mentioned here: https://www.biorxiv.org/content/10.1101/436659v1.

library(rentrez)
library(taxize)

Sys.info()
#>                              sysname                              release 
#>                              "Linux"                   "4.9.125-linuxkit" 
#>                              version                             nodename 
#> "#1 SMP Fri Sep 7 08:20:28 UTC 2018"                       "8985d6ae7da2" 
#>                              machine                                login 
#>                             "x86_64"                            "unknown" 
#>                                 user                       effective_user 
#>                            "rstudio"                            "rstudio"
sessionInfo()
#> R version 3.5.2 (2018-12-20)
#> Platform: x86_64-pc-linux-gnu (64-bit)
#> Running under: Debian GNU/Linux 9 (stretch)
#> 
#> Matrix products: default
#> BLAS: /usr/lib/openblas-base/libblas.so.3
#> LAPACK: /usr/lib/libopenblasp-r0.2.19.so
#> 
#> locale:
#>  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
#>  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
#>  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=C             
#>  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
#>  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
#> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] taxize_0.9.5  rentrez_1.2.1
#> 
#> loaded via a namespace (and not attached):
#>  [1] Rcpp_1.0.0        xml2_1.2.0        knitr_1.22       
#>  [4] magrittr_1.5      ape_5.2           lattice_0.20-38  
#>  [7] R6_2.4.0          foreach_1.4.4     plyr_1.8.4       
#> [10] stringr_1.4.0     highr_0.7         tools_3.5.2      
#> [13] parallel_3.5.2    bold_0.8.6        grid_3.5.2       
#> [16] data.table_1.12.0 nlme_3.1-137      xfun_0.5         
#> [19] iterators_1.0.10  htmltools_0.3.6   yaml_2.2.0       
#> [22] digest_0.6.18     httpcode_0.2.0    reshape2_1.4.3   
#> [25] codetools_0.2-15  curl_3.3          crul_0.7.0       
#> [28] evaluate_0.13     rmarkdown_1.11    stringi_1.4.3    
#> [31] compiler_3.5.2    XML_3.98-1.19     reshape_0.8.8    
#> [34] jsonlite_1.6      zoo_1.8-4
dwinter commented 5 years ago

Hi anyone still following this thread, this bug is now fixed. It's possible that the workarounds you ahve been using will raise a warning because rentrez now over-rules any http_option that is is given. Let me know if you have any remaining issues with this.