fix: PR changes

supertokens · Jun 4, 2024 · 6756e33 · 6756e33
1 parent 2434230
commit 6756e33
Show file tree

Hide file tree

Showing 14 changed files with 133 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changes
 
-- Fixed the session refresh loop in all the request interceptors that occurred when an API returned a 401 response despite a valid session. Interceptors now attempt to refresh the session a maximum of three times before returning the last API response. The retry limit is configurable via the `maxRetryAttemptsForSessionRefresh` option.
+- Fixed the session refresh loop in all the request interceptors that occurred when an API returned a 401 response despite a valid session. Interceptors now attempt to refresh the session a maximum of ten times before throwing an error. The retry limit is configurable via the `maxRetryAttemptsForSessionRefresh` option.
 
 ## [20.0.1] - 2024-05-24
 

diff --git a/bundle/bundle.js b/bundle/bundle.js
diff --git a/lib/build/axios.js b/lib/build/axios.js
diff --git a/lib/build/fetch.js b/lib/build/fetch.js
diff --git a/lib/build/types.d.ts b/lib/build/types.d.ts
diff --git a/lib/build/utils/index.js b/lib/build/utils/index.js
diff --git a/lib/build/xmlhttprequest.js b/lib/build/xmlhttprequest.js
diff --git a/lib/ts/axios.ts b/lib/ts/axios.ts
@@ -473,7 +473,10 @@ export default class AuthHttpRequest {
                                 logDebugMessage(
                                     `doRequest: Maximum session refresh attempts reached. sessionRefreshAttempts: ${config.__supertokensSessionRefreshAttempts}, maxRetryAttemptsForSessionRefresh: ${AuthHttpRequestFetch.config.maxRetryAttemptsForSessionRefresh}`
                                 );
-                                return response;
+
+                                const errorMessage = `Received a 401 response from ${url}. Attempted to refresh the session and retry the request with the updated session tokens ${AuthHttpRequestFetch.config.maxRetryAttemptsForSessionRefresh} times, but each attempt resulted in a 401 error. The maximum session refresh limit has been reached. Please investigate your API. To increase the session refresh attempts, update maxRetryAttemptsForSessionRefresh in the config.`;
+                                console.error(errorMessage);
+                                throw new Error(errorMessage);
                             }
 
                             const refreshResult = await onUnauthorisedResponse(preRequestLSS);

diff --git a/lib/ts/fetch.ts b/lib/ts/fetch.ts
@@ -352,7 +352,10 @@ export default class AuthHttpRequest {
                         logDebugMessage(
                             `doRequest: Maximum session refresh attempts reached. sessionRefreshAttempts: ${sessionRefreshAttempts}, maxRetryAttemptsForSessionRefresh: ${AuthHttpRequest.config.maxRetryAttemptsForSessionRefresh}`
                         );
-                        return response;
+
+                        const errorMessage = `Received a 401 response from ${url}. Attempted to refresh the session and retry the request with the updated session tokens ${AuthHttpRequest.config.maxRetryAttemptsForSessionRefresh} times, but each attempt resulted in a 401 error. The maximum session refresh limit has been reached. Please investigate your API. To increase the session refresh attempts, update maxRetryAttemptsForSessionRefresh in the config.`;
+                        console.error(errorMessage);
+                        throw new Error(errorMessage);
                     }
 
                     let retry = await onUnauthorisedResponse(preRequestLSS);

diff --git a/lib/ts/types.ts b/lib/ts/types.ts
@@ -58,7 +58,7 @@ export type InputType = {
      * This specifies the maximum number of times the interceptor will attempt to refresh
      * the session  when a 401 Unauthorized response is received. If the number of retries
      * exceeds this limit, no further attempts will be made to refresh the session, and
-     * the last response will be returned to the caller.
+     * and an error will be thrown.
      */
     maxRetryAttemptsForSessionRefresh?: number;
     tokenTransferMethod?: "cookie" | "header";

diff --git a/lib/ts/utils/index.ts b/lib/ts/utils/index.ts
@@ -116,7 +116,7 @@ export function validateAndNormaliseInputOrThrowError(options: InputType): Norma
         sessionTokenBackendDomain = normaliseSessionScopeOrThrowError(options.sessionTokenBackendDomain);
     }
 
-    let maxRetryAttemptsForSessionRefresh = 3;
+    let maxRetryAttemptsForSessionRefresh = 10;
     if (options.maxRetryAttemptsForSessionRefresh !== undefined) {
         if (options.maxRetryAttemptsForSessionRefresh < 0) {
             throw new Error("maxRetryAttemptsForSessionRefresh must be greater than or equal to 0.");

diff --git a/lib/ts/xmlhttprequest.ts b/lib/ts/xmlhttprequest.ts
@@ -136,7 +136,15 @@ export function addInterceptorsToXMLHttpRequest() {
                 logDebugMessage(
                     `XHRInterceptor.handleRetryPostRefreshing: Maximum session refresh attempts reached. sessionRefreshAttempts: ${sessionRefreshAttempts}, maxRetryAttemptsForSessionRefresh: ${AuthHttpRequestFetch.config.maxRetryAttemptsForSessionRefresh}`
                 );
-                return true;
+
+                // We set these values to prevent XHR from returning any response in this case. This simulates a network error in XHR.
+                customGetterValues["status"] = 0;
+                customGetterValues["statusText"] = "";
+                customGetterValues["responseType"] = "";
+
+                const errorMessage = `Received a 401 response from ${url}. Attempted to refresh the session and retry the request with the updated session tokens ${AuthHttpRequestFetch.config.maxRetryAttemptsForSessionRefresh} times, but each attempt resulted in a 401 error. The maximum session refresh limit has been reached. Please investigate your API. To increase the session refresh attempts, update maxRetryAttemptsForSessionRefresh in the config.`;
+                console.error(errorMessage);
+                throw new Error(errorMessage);
             }
 
             const refreshResult = await onUnauthorisedResponse(preRequestLSS);
@@ -240,7 +248,12 @@ export function addInterceptorsToXMLHttpRequest() {
                 } else {
                     // Here we only need to handle fetch related errors, from the refresh endpoint called by the retry
                     // So we should only get network level errors here
-                    redispatchEvent("error", new Event("error"));
+
+                    const ev = new Event("error");
+                    (ev as any).error = err;
+                    self.dispatchEvent(ev);
+
+                    redispatchEvent("error", ev);
                 }
                 return true;
             }

diff --git a/test/cross.auto_refresh.test.js b/test/cross.auto_refresh.test.js
@@ -667,6 +667,12 @@ addTestCases((name, transferMethod, setupFunc, setupArgs = []) => {
             await startST();
             await setup();
             await page.setRequestInterception(true);
+
+            let consoleLogs = [];
+            page.on("console", message => {
+                consoleLogs.push(message.text());
+            });
+
             let count = 0;
             page.on("request", req => {
                 const url = req.url();
@@ -704,24 +710,33 @@ addTestCases((name, transferMethod, setupFunc, setupArgs = []) => {
                 //check that the number of times the refreshAPI was called is 0
                 assert.strictEqual(await getNumberOfTimesRefreshCalled(), 0);
 
-                let getResponse = await toTest({ url: `${BASE_URL}/` });
-
-                assert.strictEqual(getResponse.statusCode, 401);
+                await assert.rejects(async () => {
+                    await toTest({ url: `${BASE_URL}/` });
+                });
 
                 const numRefreshCalled = await getNumberOfTimesRefreshCalled();
+                assert.strictEqual(numRefreshCalled, 10);
+            });
 
-                assert.strictEqual(numRefreshCalled, 3);
+            assert(
+                consoleLogs.includes(
+                    "Received a 401 response from http://localhost.org:8080/. Attempted to refresh the session and retry the request with the updated session tokens 10 times, but each attempt resulted in a 401 error. The maximum session refresh limit has been reached. Please investigate your API. To increase the session refresh attempts, update maxRetryAttemptsForSessionRefresh in the config."
+                )
+            );
 
-                // Check that the last response was returned
-                assert.strictEqual(JSON.parse(getResponse.responseText).count, numRefreshCalled + 1);
-            });
             await page.setRequestInterception(false);
         });
 
         it("should break out of session refresh loop after configured maxRetryAttemptsForSessionRefresh value", async function () {
             await startST();
             await setup();
             await page.setRequestInterception(true);
+
+            let consoleLogs = [];
+            page.on("console", message => {
+                consoleLogs.push(message.text());
+            });
+
             let count = 0;
             page.on("request", req => {
                 const url = req.url();
@@ -740,7 +755,7 @@ addTestCases((name, transferMethod, setupFunc, setupArgs = []) => {
             await page.evaluate(async () => {
                 supertokens.init({
                     apiDomain: BASE_URL,
-                    maxRetryAttemptsForSessionRefresh: 10
+                    maxRetryAttemptsForSessionRefresh: 5
                 });
 
                 const userId = "testing-supertokens-website";
@@ -760,24 +775,33 @@ addTestCases((name, transferMethod, setupFunc, setupArgs = []) => {
                 //check that the number of times the refreshAPI was called is 0
                 assert.strictEqual(await getNumberOfTimesRefreshCalled(), 0);
 
-                let getResponse = await toTest({ url: `${BASE_URL}/` });
-
-                assert.strictEqual(getResponse.statusCode, 401);
+                await assert.rejects(async () => {
+                    await toTest({ url: `${BASE_URL}/` });
+                });
 
                 const numRefreshCalled = await getNumberOfTimesRefreshCalled();
+                assert.strictEqual(numRefreshCalled, 5);
+            });
 
-                assert.strictEqual(numRefreshCalled, 10);
+            assert(
+                consoleLogs.includes(
+                    "Received a 401 response from http://localhost.org:8080/. Attempted to refresh the session and retry the request with the updated session tokens 5 times, but each attempt resulted in a 401 error. The maximum session refresh limit has been reached. Please investigate your API. To increase the session refresh attempts, update maxRetryAttemptsForSessionRefresh in the config."
+                )
+            );
 
-                // Check that the last response was returned
-                assert.strictEqual(JSON.parse(getResponse.responseText).count, numRefreshCalled + 1);
-            });
             await page.setRequestInterception(false);
         });
 
         it("should not do session refresh if maxRetryAttemptsForSessionRefresh is 0", async function () {
             await startST();
             await setup();
             await page.setRequestInterception(true);
+
+            let consoleLogs = [];
+            page.on("console", message => {
+                consoleLogs.push(message.text());
+            });
+
             let count = 0;
             page.on("request", req => {
                 const url = req.url();
@@ -816,17 +840,20 @@ addTestCases((name, transferMethod, setupFunc, setupArgs = []) => {
                 //check that the number of times the refreshAPI was called is 0
                 assert.strictEqual(await getNumberOfTimesRefreshCalled(), 0);
 
-                let getResponse = await toTest({ url: `${BASE_URL}/` });
-
-                assert.strictEqual(getResponse.statusCode, 401);
+                await assert.rejects(async () => {
+                    await toTest({ url: `${BASE_URL}/` });
+                });
 
                 const numRefreshCalled = await getNumberOfTimesRefreshCalled();
-
                 assert.strictEqual(numRefreshCalled, 0);
-
-                // Check that the last response was returned
-                assert.strictEqual(JSON.parse(getResponse.responseText).count, numRefreshCalled + 1);
             });
+
+            assert(
+                consoleLogs.includes(
+                    "Received a 401 response from http://localhost.org:8080/. Attempted to refresh the session and retry the request with the updated session tokens 0 times, but each attempt resulted in a 401 error. The maximum session refresh limit has been reached. Please investigate your API. To increase the session refresh attempts, update maxRetryAttemptsForSessionRefresh in the config."
+                )
+            );
+
             await page.setRequestInterception(false);
         });
     });