more consistent ncurl_session() behaviour

Author: shikokuchuo

NEWS.md

@@ -13,6 +13,7 @@
 
 * Send mode 'next' is folded into the default 'serial', with custom serialization functions applying automatically if they have been registered.
 * The session-wide `next_config()` is now deprecated and defunct, in favour of the new `serial_config()`.
+* `ncurl_session()` now returns 'errorValue' 7 (Object closed) when attempting to transact over a closed session or closing a closed session, rather than throwing an error.
 * `collect_aio()` and `collect_aio_()` no longer append empty names when acting on lists of Aios where there were none in the first place.
 * Removes hard dependency on `stats` and `utils` base packages.
 * Requires R >= 3.6.

R/socket.R

@@ -179,9 +179,7 @@ collect_pipe <- function(x) .Call(rnng_aio_collect_pipe, x)
 #'     will be terminated and any new operations will fail after the connection
 #'     is closed.
 #'
-#'     Closing an \sQuote{ncurlSession} closes any active http(s) connection and
-#'     frees all related resources. The session is no longer active and cannot
-#'     be re-used.
+#'     Closing an \sQuote{ncurlSession} closes the http(s) connection.
 #'
 #'     As Pipes are owned by the corresponding Socket, removing (and garbage
 #'     collecting) a Pipe does not close it or free its resources. A Pipe may,

man/close.Rd

@@ -135,8 +135,8 @@ static void session_finalizer(SEXP xptr) {
   if (NANO_PTR(xptr) == NULL) return;
   nano_aio *xp = (nano_aio *) NANO_PTR(xptr);
   nano_handle *handle = (nano_handle *) xp->next;
-  nng_http_conn *conn = (nng_http_conn *) xp->data;
-  nng_http_conn_close(conn);
+  if (xp->data != NULL)
+    nng_http_conn_close((nng_http_conn *) xp->data);
   nng_aio_free(xp->aio);
   if (handle->cfg != NULL)
     nng_tls_config_free(handle->cfg);
@@ -659,9 +659,12 @@ SEXP rnng_ncurl_transact(SEXP session) {
     Rf_error("'session' is not a valid or active ncurlSession");
 
   nano_aio *haio = (nano_aio *) NANO_PTR(session);
+
+  if (haio->data == NULL)
+    return mk_error_ncurl(7);
+
   nng_http_conn *conn = (nng_http_conn *) haio->data;
   nano_handle *handle = (nano_handle *) haio->next;
-
   nng_http_conn_transact(conn, handle->req, handle->res, haio->aio);
   nng_aio_wait(haio->aio);
   if (haio->result > 0)
@@ -713,10 +716,13 @@ SEXP rnng_ncurl_session_close(SEXP session) {
   if (NANO_TAG(session) != nano_StatusSymbol)
     Rf_error("'session' is not a valid or active ncurlSession");
 
-  session_finalizer(session);
-  NANO_SET_TAG(session, R_NilValue);
-  NANO_SET_PROT(session, R_NilValue);
-  R_ClearExternalPtr(session);
+  nano_aio *haio = (nano_aio *) NANO_PTR(session);
+
+  if (haio->data == NULL)
+    ERROR_RET(7);
+
+  nng_http_conn_close((nng_http_conn *) haio->data);
+  haio->data = NULL;
   Rf_setAttrib(session, nano_StateSymbol, R_MissingArg);
 
   return nano_success;

src/ncurl.c

@@ -12,6 +12,7 @@ nanotestxp <- function(x) invisible(typeof(x) == "externalptr" || stop("is not o
 nanotesterr <- function(x, e = "")
   invisible(grepl(e, tryCatch(x, error = identity)[["message"]], fixed = TRUE) || stop("expected error message '", e, "' not generated"))
 later <- requireNamespace("later", quietly = TRUE)
+promises <- requireNamespace("promises", quietly = TRUE)
 
 nng_version()
 nanotestnano(n <- nano("req", listen = "inproc://nanonext", autostart = FALSE))
@@ -408,11 +409,12 @@ sess <- ncurl_session("https://postman-echo.com/post", method = "POST", headers
 nanotest(is_ncurl_session(sess) || is_error_value(sess))
 if (is_ncurl_session(sess)) nanotest(length(transact(sess)) == 3L)
 if (is_ncurl_session(sess)) nanotest(close(sess) == 0L)
+if (is_ncurl_session(sess)) nanotestw(close(sess) == 7L)
 sess <- ncurl_session("https://postman-echo.com/post", convert = FALSE, method = "POST", headers = c(`Content-Type` = "text/plain"), timeout = 3000)
 nanotest(is_ncurl_session(sess) || is_error_value(sess))
 if (is_ncurl_session(sess)) nanotest(length(transact(sess)) == 3L)
 if (is_ncurl_session(sess)) nanotest(close(sess) == 0L)
-nanotesterr(transact(sess), "ncurlSession")
+if (is_ncurl_session(sess)) nanotest(transact(sess)$data == 7L)
 nanotestw(is_error_value(ncurl_session("https://i")))
 nanotesterr(ncurl_aio("https://", tls = "wrong"), "valid TLS")
 nanotesterr(ncurl("https://www.cam.ac.uk/", tls = "wrong"), "valid TLS")
@@ -595,13 +597,11 @@ nanotestw(listen(s, url = "tls+tcp://.", tls = tls, error = FALSE) > 0)
 nanotestz(close(s1))
 nanotestz(close(s))
 nanotest(!identical(get0(".Random.seed"), {.advance(); .Random.seed}))
-if (requireNamespace("promises", quietly = TRUE)) {
-  nanotestaio(n <- ncurl_aio("https://postman-echo.com/get"))
-  nanotest(tryCatch(promises::is.promise(promises::then(n, cat)), error = function(e) TRUE))
-  nanotest(promises::is.promising(call_aio(n)))
-  nanotest(promises::is.promise(promises::as.promise(call_aio(ncurl_aio("https://postman-echo.com/get")))))
-  later::run_now()
-}
+if (promises) nanotestaio(n <- ncurl_aio("https://postman-echo.com/get"))
+if (promises) nanotest(tryCatch(promises::is.promise(promises::then(n, cat)), error = function(e) TRUE))
+if (promises) nanotest(promises::is.promising(call_aio(n)))
+if (promises) nanotest(promises::is.promise(promises::as.promise(call_aio(ncurl_aio("https://postman-echo.com/get")))))
+if (promises) later::run_now()
 if (Sys.info()[["sysname"]] == "Linux") {
   rm(list = ls())
   gc()