Skip to content

fix: propagate server error messages instead of swallowing them#298

Merged
DevOpsDave merged 2 commits intomasterfrom
fix/propagate-server-error-message
Apr 17, 2026
Merged

fix: propagate server error messages instead of swallowing them#298
DevOpsDave merged 2 commits intomasterfrom
fix/propagate-server-error-message

Conversation

@DevOpsDave
Copy link
Copy Markdown
Contributor

Summary

  • Bug: The catch block in getIamKey.ts (line 146-148) discards the actual server error from alks.getIAMKeys() and always throws a generic "Incorrect account or role" message. This prevents meaningful server-side errors from reaching the user.
  • Fix: Propagate the server's error message when available, falling back to badAccountMessage only when no message is present.
  • Tests: Updated existing test case and added a new test case verifying that server error messages (e.g., "Change request required for production access") are properly surfaced.

Context

Starting the week of April 20, 2026, ALKS Core will enforce ChangeMinder (change ticket requirements) for production access via an Optimizely feature flag. When CLI users try to get prod keys without providing --ciid or --chg-number, the server returns a meaningful rejection message — but the CLI currently swallows it and shows "Incorrect account or role", which will confuse users and flood #alks-support.

Changes

src/lib/getIamKey.ts

  } catch (e) {
-   throw new Error(badAccountMessage);
+   const serverMessage = (e as Error).message;
+   throw new Error(serverMessage || badAccountMessage);
  }

src/lib/getIamKey.test.ts

  • Renamed existing "when alks.getIAMKeys fails" test to "when alks.getIAMKeys fails with no message" and added assertion that it falls back to badAccountMessage
  • Added new test case "when alks.getIAMKeys fails with a server error message" that verifies the server message is propagated

Test plan

  • npm test passes — all 44 tests in getIamKey.test.ts pass
  • Manual test: run alks sessions open against a prod account without --ciid/--chg-number after ChangeMinder enforcement is enabled — should see the server's rejection message instead of "Incorrect account or role"

🤖 Generated with Claude Code

…owing them

The catch block in getIamKey was discarding the actual server error and
always throwing a generic "Incorrect account or role" message. This
prevented meaningful server-side errors (e.g., ChangeMinder enforcement
rejections) from reaching the user. Now the server's error message is
propagated when available, falling back to the generic message only when
no message is present.
@DevOpsDave DevOpsDave added the release/patch Indicates an update without breaking changes or new features label Apr 17, 2026
@samsolaimani samsolaimani self-requested a review April 17, 2026 20:53
Copy link
Copy Markdown
Contributor

@samsolaimani samsolaimani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a well-scoped, correct fix with good test coverage. Approved 👍

Copy link
Copy Markdown
Contributor

@americk0 americk0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple enough. 👍

@DevOpsDave DevOpsDave merged commit 50d21f9 into master Apr 17, 2026
5 checks passed
@DevOpsDave DevOpsDave deleted the fix/propagate-server-error-message branch April 17, 2026 21:06
@DevOpsDave DevOpsDave mentioned this pull request Apr 20, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/patch Indicates an update without breaking changes or new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants