From 9ad2acae7ca84623e41a00ae8eb4464d2059c392 Mon Sep 17 00:00:00 2001
From: Shawn Garbett <shawn.garbett@vumc.org>
Date: Tue, 26 Nov 2024 10:43:09 -0600
Subject: [PATCH 1/3] Improved error messages #413

---
 NEWS                                   | 1 +
 R/unlockREDCap.R                       | 8 ++++++--
 tests/testthat/test-024-unlockREDCap.R | 7 ++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 2f2e73b1..ac092bd2 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@ A future release of version 3.0.0 will introduce several breaking changes!
 * `unlockREDCap` no longer changes console focus
 * Vectorized `renameRecord` and `exportFieldNames`
 * Exporting `connectAndCheck` function to establish connections
+* Improved error messages when misspecified URL is provided.
 
 ## 2.10.0
 
diff --git a/R/unlockREDCap.R b/R/unlockREDCap.R
index 9560c1fa..56d35ad1 100644
--- a/R/unlockREDCap.R
+++ b/R/unlockREDCap.R
@@ -59,13 +59,17 @@ connectAndCheck <- function(key, url, ...)
       if(response$status_code %in% c(301L, 302L))
         stop(paste("Too many redirects from", url))
       
+
       rcon
     },
     error = function(e)
     {
-      if(grepl("Could not resolve host",     e) ||
+      if(grepl("Could not resolve host",     e)  ||
          grepl("Could not connect to server", e))
-        stop("Unable to connect to url '",url,"'. ", e$message)
+        stop("Invalid URL provided '",url,"'. Unable to resolve or route.\n", e$message)
+      
+      if(grepl("405", e$message) )
+        stop("URL '",url,"' refused connection. Not acting like a REDCap server.\n", e$message)
         
       if(grepl("403", e)) return(NULL) # Forbidden, i.e. bad API_KEY
       
diff --git a/tests/testthat/test-024-unlockREDCap.R b/tests/testthat/test-024-unlockREDCap.R
index 9256ce00..abddaba4 100644
--- a/tests/testthat/test-024-unlockREDCap.R
+++ b/tests/testthat/test-024-unlockREDCap.R
@@ -69,7 +69,12 @@ test_that(
 
 test_that(
   "connectAndCheck errors with bad url",
-  expect_error(connectAndCheck("key", "badurl"), "Unable to connect")
+  expect_error(connectAndCheck("key", "badurl"), "Invalid URL provided")
+)
+
+test_that(
+  "connectAndCheck errors with valid url but not a REDCap server",
+  expect_error(connectAndCheck("key", "https://google.com"), "refused connection")
 )
 
 test_that(

From 436b3f46737c2529329f26ef088e0510079ef239 Mon Sep 17 00:00:00 2001
From: Shawn Garbett <shawn.garbett@vumc.org>
Date: Tue, 26 Nov 2024 12:14:20 -0600
Subject: [PATCH 2/3] Fix for redirect URL construction #413

---
 R/unlockREDCap.R                       | 5 ++++-
 tests/testthat/test-024-unlockREDCap.R | 9 +++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/R/unlockREDCap.R b/R/unlockREDCap.R
index 56d35ad1..e42d2727 100644
--- a/R/unlockREDCap.R
+++ b/R/unlockREDCap.R
@@ -50,7 +50,10 @@ connectAndCheck <- function(key, url, ...)
       if(!response$status_code %in% c(301L, 302L)) return(rcon)
       
       # Handle redirect
-      rcon <- redcapConnection(token=key, url=response$header$location, ...)
+      rcon <- redcapConnection(
+        token = key,
+        url   = paste0(response$header$location, '/api/'),
+        ...)
       
       # Test connection by checking version post redirect
       response <- makeApiCall(rcon, body = version,
diff --git a/tests/testthat/test-024-unlockREDCap.R b/tests/testthat/test-024-unlockREDCap.R
index abddaba4..5b0936fa 100644
--- a/tests/testthat/test-024-unlockREDCap.R
+++ b/tests/testthat/test-024-unlockREDCap.R
@@ -5,12 +5,12 @@ library(curl)
 
 h <- new_handle(timeout = 1L)
 redirect <- structure(
-  list(url = "https://test.xyz/api",
+  list(url = "https://test.xyz/api/",
        status_code = 302L,
        content = "",
        headers=structure(list(
          'content-type'="text/csv; charset=utf-8",
-         'location'=url
+         'location'=gsub('\\/api\\/', '', url)
        ),
        class = c("insensitive", "list")),
   class = "response")
@@ -29,10 +29,11 @@ test_that(
   "connectAndCheck deals with redirect 301 status",
   {
     redirectCall <- TRUE
+    redirect$status_code = 301L
     stub(connectAndCheck, "makeApiCall", function(...)
       if(redirectCall) { redirectCall <<- FALSE; redirect  } else {makeApiCall(...)})
 
-    rcon <- connectAndCheck(rcon$token, "https://test.xyz/api")
+    rcon <- connectAndCheck(rcon$token, "https://test.xyz/api/")
     expect_equal(rcon$url, url)
   }
 )
@@ -44,7 +45,7 @@ test_that(
     stub(connectAndCheck, "makeApiCall", function(...)
       if(redirectCall) { redirectCall <<- FALSE; redirect  } else {makeApiCall(...)})
 
-    rcon <- connectAndCheck(rcon$token, "https://test.xyz/api")
+    rcon <- connectAndCheck(rcon$token, "https://test.xyz/api/")
     expect_equal(rcon$url, url)
   }
 )

From a44b68e31116b014dc4894fcc09a8978b920a54a Mon Sep 17 00:00:00 2001
From: Shawn Garbett <shawn.garbett@vumc.org>
Date: Wed, 4 Dec 2024 14:09:12 -0600
Subject: [PATCH 3/3] NEWS for #413

---
 NEWS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS b/NEWS
index ac092bd2..0997ff19 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,7 @@ A future release of version 3.0.0 will introduce several breaking changes!
 * Vectorized `renameRecord` and `exportFieldNames`
 * Exporting `connectAndCheck` function to establish connections
 * Improved error messages when misspecified URL is provided.
+* Fix for redirected URLs
 
 ## 2.10.0