Skip to content

fix(BA-5559): handle non-dict data field in login response#10732

Open
agatha197 wants to merge 4 commits intomainfrom
bugfix/BA-5559-fix-login-cmd-attributeerror
Open

fix(BA-5559): handle non-dict data field in login response#10732
agatha197 wants to merge 4 commits intomainfrom
bugfix/BA-5559-fix-login-cmd-attributeerror

Conversation

@agatha197
Copy link
Copy Markdown
Contributor

@agatha197 agatha197 commented Apr 2, 2026

Resolves BA-5559

Summary

  • Fix AttributeError in login_cmd.py when the server returns a string in the data field instead of a dict
  • Add isinstance checks before calling .get() on the response data field
  • Handles cases like "An active login session already exists" where data is a plain string

Test plan

  • Run ./bai login when an active session exists — should show a clear error message instead of crashing
  • Run ./bai login --force — should override existing session successfully
  • Normal login flow still works as expected

Copilot AI review requested due to automatic review settings April 2, 2026 02:58
@github-actions github-actions bot added size:S 10~30 LoC comp:client Related to Client component comp:cli Related to CLI component labels Apr 2, 2026
The login CLI assumed the server always returns a dict in the
response data field. When an active session exists, the server
returns a plain string, causing an AttributeError on .get().
Add isinstance checks to handle both string and dict responses.
@agatha197 agatha197 force-pushed the bugfix/BA-5559-fix-login-cmd-attributeerror branch from 478b11d to 30f5986 Compare April 2, 2026 02:59
@agatha197 agatha197 requested a review from HyeockJinKim April 2, 2026 03:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a crash in the v2 CLI login flow when the server returns a non-dict data field (e.g., a plain string message), by safely extracting an error detail string before calling .get().

Changes:

  • Add isinstance(..., dict) guards when reading data["data"]["details"] during login failure handling.
  • Improve resilience of OTP prompting logic by handling non-dict data payloads without raising AttributeError.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@agatha197 agatha197 requested a review from fregataa April 2, 2026 03:00
@agatha197 agatha197 marked this pull request as draft April 2, 2026 03:04
@agatha197 agatha197 requested a review from Copilot April 2, 2026 03:05
@agatha197 agatha197 marked this pull request as ready for review April 2, 2026 03:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@agatha197 agatha197 requested a review from fregataa April 2, 2026 05:30
@fregataa
Copy link
Copy Markdown
Member

fregataa commented Apr 2, 2026

Please check lint and typecheck failure

05:35:47.35 [ERROR] Completed: Typecheck using MyPy - mypy - mypy failed (exit code 1).
src/ai/backend/client/cli/v2/login_cmd.py:51: error: Name "Any" is not defined  [name-defined]
src/ai/backend/client/cli/v2/login_cmd.py:51: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Any")
Found 1 error in 1 file (checked 1029 source files)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:cli Related to CLI component comp:client Related to Client component size:S 10~30 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants