breaking change: do not throw IDX responses as errors

OKTA-481844 <<<Jenkins Check-In of Tested SHA: c2ae026 for eng_productivity_ci_bot_okta@okta.com>>> Artifact: okta-auth-js Files changed count: 15 PR Link: #1205
okta · Sep 21, 2022 · 13bbcea · 13bbcea
1 parent 8c62df8
commit 13bbcea
Show file tree

Hide file tree

Showing 16 changed files with 169 additions and 186 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -13,3 +13,5 @@ node_modules
 .eslintrc.js
 dist
 target
+/test/apps/app/public/oidc-app.js
+/test/apps/app/target
diff --git a/lib/idx/idxState/v1/generateIdxAction.ts b/lib/idx/idxState/v1/generateIdxAction.ts
@@ -64,8 +64,7 @@ const generateDirectFetch = function generateDirectFetch(authClient: OktaAuthIdx
         idxResponse.stepUp = true;
       }
 
-      // Throw IDX response if request did not succeed. This behavior will be removed in version 7.0: OKTA-481844
-      throw idxResponse;
+      return idxResponse;
     }
   };
 };

diff --git a/lib/idx/register.ts b/lib/idx/register.ts
@@ -34,14 +34,10 @@ export async function register(
       autoRemediate: false
     });
     if (!options.activationToken && enabledFeatures && !enabledFeatures.includes(IdxFeature.REGISTRATION)) {
-      const error = new AuthSdkError('Registration is not supported based on your current org configuration.');
-      throw error;
-    // return { status: IdxStatus.FAILURE, error } as unknown as IdxTransaction; // TODO: wny not just throw the error?
+      throw new AuthSdkError('Registration is not supported based on your current org configuration.');
     }
     if (options.activationToken && availableSteps?.some(({ name }) => name === 'identify')) {
-      const error = new AuthSdkError('activationToken is not supported based on your current org configuration.');
-      throw error;
-    // return { status: IdxStatus.FAILURE, error } as unknown as IdxTransaction; // TODO: wny not just throw the error?
+      throw new AuthSdkError('activationToken is not supported based on your current org configuration.');
     }
   }
 

diff --git a/lib/idx/remediate.ts b/lib/idx/remediate.ts
@@ -25,7 +25,7 @@ import {
   filterValuesForRemediation,
   getRemediator,
   getNextStep,
-  handleIdxError
+  handleFailedResponse
 } from './util';
 
 export interface RemediateActionWithOptionalParams {
@@ -97,11 +97,9 @@ export async function remediate(
       let optionsWithoutExecutedAction = removeActionFromOptions(options, action);
 
       if (typeof idxResponse.actions[action] === 'function') {
-        try {
-          idxResponse = await idxResponse.actions[action](params);
-          idxResponse = { ...idxResponse, requestDidSucceed: true };
-        } catch (e) {
-          return handleIdxError(authClient, e, options);
+        idxResponse = await idxResponse.actions[action](params);
+        if (idxResponse.requestDidSucceed === false) {
+          return handleFailedResponse(authClient, idxResponse, options);
         }
         if (action === 'cancel') {
           return { idxResponse, canceled: true };
@@ -117,14 +115,10 @@ export async function remediate(
       // search for action in remediation list
       const remediationAction = neededToProceed.find(({ name }) => name === action);
       if (remediationAction) {
-        try {
-          idxResponse = await idxResponse.proceed(action, params);
-          idxResponse = { ...idxResponse, requestDidSucceed: true };
+        idxResponse = await idxResponse.proceed(action, params);
+        if (idxResponse.requestDidSucceed === false) {
+          return handleFailedResponse(authClient, idxResponse, options);
         }
-        catch (e) {
-          return handleIdxError(authClient, e, options);
-        }
-
         return remediate(authClient, idxResponse, values, optionsWithoutExecutedAction); // recursive call
       }
     }
@@ -137,16 +131,17 @@ export async function remediate(
   }
 
   if (!remediator) {
+    // With options.step, remediator is not required
     if (options.step) {
       values = filterValuesForRemediation(idxResponse, options.step, values); // include only requested values
-      try {
-        idxResponse = await idxResponse.proceed(options.step, values);
-        idxResponse = { ...idxResponse, requestDidSucceed: true };
-        return { idxResponse };
-      } catch(e) {
-        return handleIdxError(authClient, e, options);
+      idxResponse = await idxResponse.proceed(options.step, values);
+      if (idxResponse.requestDidSucceed === false) {
+        return handleFailedResponse(authClient, idxResponse, options);
       }
+      return { idxResponse };
     }
+
+    // With default flow, remediator is not required
     if (flow === 'default') {
       return { idxResponse };
     }
@@ -167,28 +162,28 @@ export async function remediate(
 
   const name = remediator.getName();
   const data = remediator.getData();
-  try {
-    idxResponse = await idxResponse.proceed(name, data);
-    idxResponse = { ...idxResponse, requestDidSucceed: true };
-    // We may want to trim the values bag for the next remediation
-    // Let the remediator decide what the values should be (default to current values)
-    values = remediator.getValuesAfterProceed();
-    options = { ...options, step: undefined }; // do not re-use the step
-
-    // generic remediator should not auto proceed in pending status
-    // return nextStep directly
-    if (options.useGenericRemediator && !idxResponse.interactionCode && !isTerminalResponse(idxResponse)) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      const gr = getRemediator(idxResponse.neededToProceed, values, options)!;
-      const nextStep = getNextStep(authClient, gr, idxResponse);
-      return {
-        idxResponse,
-        nextStep,
-      };
-    }
-
-    return remediate(authClient, idxResponse, values, options); // recursive call
-  } catch (e) {
-    return handleIdxError(authClient, e, options);
+
+  idxResponse = await idxResponse.proceed(name, data);
+  if (idxResponse.requestDidSucceed === false) {
+    return handleFailedResponse(authClient, idxResponse, options);
   }
+  // We may want to trim the values bag for the next remediation
+  // Let the remediator decide what the values should be (default to current values)
+  values = remediator.getValuesAfterProceed();
+  options = { ...options, step: undefined }; // do not re-use the step
+
+  // generic remediator should not auto proceed in pending status
+  // return nextStep directly
+  if (options.useGenericRemediator && !idxResponse.interactionCode && !isTerminalResponse(idxResponse)) {
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const gr = getRemediator(idxResponse.neededToProceed, values, options)!;
+    const nextStep = getNextStep(authClient, gr, idxResponse);
+    return {
+      idxResponse,
+      nextStep,
+    };
+  }
+
+  return remediate(authClient, idxResponse, values, options); // recursive call
+
 }
diff --git a/lib/idx/run.ts b/lib/idx/run.ts
@@ -29,7 +29,7 @@ import {
   Tokens,
   APIError,
 } from '../types';
-import { IdxMessage, IdxResponse, isIdxResponse } from './types/idx-js';
+import { IdxMessage, IdxResponse } from './types/idx-js';
 import { getSavedTransactionMeta, saveTransactionMeta } from './transactionMeta';
 import { getAvailableSteps, getEnabledFeatures, getMessagesFromResponse, isTerminalResponse } from './util';
 declare interface RunData {
@@ -298,22 +298,6 @@ async function finalizeData(authClient, data: RunData): Promise<RunData> {
   };
 }
 
-function handleError(err, data: RunData): RunData {
-  let { error, status, shouldClearTransaction } = data;
-
-  // current version of idx-js will throw/reject IDX responses. Handle these differently than regular errors
-  if (isIdxResponse(err)) {
-    error = err;
-    status = IdxStatus.FAILURE;
-    shouldClearTransaction = true;
-  } else {
-    // error is not an IDX response, throw it like a regular error
-    throw err;
-  }
-
-  return { ...data, error, status, shouldClearTransaction };
-}
-
 export async function run(
   authClient: OktaAuthIdxInterface, 
   options: RunOptions = {},
@@ -324,12 +308,8 @@ export async function run(
   };
 
   data = initializeData(authClient, data);
-  try {
-    data = await getDataFromIntrospect(authClient, data);
-    data = await getDataFromRemediate(authClient, data);
-  } catch (err) {
-    data = handleError(err, data);
-  }
+  data = await getDataFromIntrospect(authClient, data);
+  data = await getDataFromRemediate(authClient, data);
   data = await finalizeData(authClient, data);
 
   const {

diff --git a/lib/idx/util.ts b/lib/idx/util.ts
@@ -3,7 +3,7 @@ import * as remediators from './remediators';
 import { RemediationValues, Remediator, RemediatorConstructor } from './remediators';
 import { GenericRemediator } from './remediators/GenericRemediator';
 import { IdxFeature, NextStep, RemediateOptions, RemediationResponse, RunOptions } from './types';
-import { IdxMessage, IdxRemediation, IdxRemediationValue, IdxResponse, isIdxResponse } from './types/idx-js';
+import { IdxMessage, IdxRemediation, IdxRemediationValue, IdxResponse } from './types/idx-js';
 import { OktaAuthIdxInterface } from '../types';
 
 export function isTerminalResponse(idxResponse: IdxResponse) {
@@ -275,17 +275,11 @@ export function getNextStep(
   };
 }
 
-export function handleIdxError(authClient: OktaAuthIdxInterface, e, options = {}): RemediationResponse {
-  // Handle idx messages
-  let idxResponse = isIdxResponse(e) ? e : null;
-  if (!idxResponse) {
-    // Thrown error terminates the interaction with idx
-    throw e;
-  }
-  idxResponse = {
-    ...idxResponse,
-    requestDidSucceed: false
-  };
+export function handleFailedResponse(
+  authClient: OktaAuthIdxInterface,
+  idxResponse: IdxResponse,
+  options = {}
+): RemediationResponse {
   const terminal = isTerminalResponse(idxResponse);
   const messages = getMessagesFromResponse(idxResponse, options);
   if (terminal) {

diff --git a/scripts/lint.sh b/scripts/lint.sh
@@ -5,9 +5,14 @@ source ${OKTA_HOME}/${REPO}/scripts/setup.sh
 export TEST_SUITE_TYPE="checkstyle"
 export TEST_RESULT_FILE_DIR="${REPO}/build2"
 
+if ! yarn tsd; then
+  echo "tsd failed! Exiting..."
+  exit ${PUBLISH_TYPE_AND_RESULT_DIR_BUT_ALWAYS_FAIL}
+fi
+
 if ! yarn lint:report; then
   echo "lint failed! Exiting..."
-  exit ${TEST_FAILURE}
+  exit ${PUBLISH_TYPE_AND_RESULT_DIR_BUT_ALWAYS_FAIL}
 fi
 
 if ! yarn workspace @okta/test.app validate; then

diff --git a/scripts/unit.sh b/scripts/unit.sh
@@ -4,10 +4,12 @@ source ${OKTA_HOME}/${REPO}/scripts/setup.sh "v14.18.0"
 
 export TEST_SUITE_TYPE="junit"
 export TEST_RESULT_FILE_DIR="${REPO}/build2/reports/unit"
+echo ${TEST_SUITE_TYPE} > ${TEST_SUITE_TYPE_FILE}
+echo ${TEST_RESULT_FILE_DIR} > ${TEST_RESULT_FILE_DIR_FILE}
 
 if ! yarn test:unit; then
   echo "unit failed! Exiting..."
-  exit ${TEST_FAILURE}
+  exit ${PUBLISH_TYPE_AND_RESULT_DIR_BUT_ALWAYS_FAIL}
 fi
 
 if ! yarn test:types; then
@@ -17,19 +19,17 @@ fi
 
 if ! yarn test:bundle:esm:browser; then
   echo "validate ESM browser bundle failed! Exiting..."
-  exit ${TEST_FAILURE}
+  exit ${PUBLISH_TYPE_AND_RESULT_DIR_BUT_ALWAYS_FAIL}
 fi
 
 if ! yarn test:bundle:esm:node; then
   echo "validate ESM node bundle failed! Exiting..."
-  exit ${TEST_FAILURE}
+  exit ${PUBLISH_TYPE_AND_RESULT_DIR_BUT_ALWAYS_FAIL}
 fi
 
 if ! yarn test:bundle:cjs; then
   echo "validate cjs bundle failed! Exiting..."
-  exit ${TEST_FAILURE}
+  exit ${PUBLISH_TYPE_AND_RESULT_DIR_BUT_ALWAYS_FAIL}
 fi
 
-echo ${TEST_SUITE_TYPE} > ${TEST_SUITE_TYPE_FILE}
-echo ${TEST_RESULT_FILE_DIR} > ${TEST_RESULT_FILE_DIR_FILE}
 exit ${PUBLISH_TYPE_AND_RESULT_DIR}
diff --git a/test/apps/app/.eslintignore b/test/apps/app/.eslintignore
@@ -1 +1,2 @@
-/public
+/public
+/target
-Original file line number
+Diff line change
@@ -1 +1,2 @@
-    /public
+    /public
+    /target