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

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

Closed
hammer opened this issue Oct 1, 2018 · 3 comments
Closed

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

hammer opened this issue Oct 1, 2018 · 3 comments

Comments

@hammer
Copy link

hammer commented Oct 1, 2018

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. curl/curl#804 (comment)).

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
Copy link
Member

dwinter commented Oct 4, 2018

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
Copy link

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
Copy link
Member

dwinter commented May 2, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants