Skip to content

qrlogin: fix Export method to handle all AuthExportLoginToken response types #1571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

fr0ster
Copy link

@fr0ster fr0ster commented Jun 13, 2025

Problem

The Export method in qrlogin package only handled *tg.AuthLoginToken response type from AuthExportLoginToken API call, but the API can return other types like *tg.AuthLoginTokenSuccess and *tg.AuthLoginTokenMigrateTo, which would cause "unexpected type" errors.

Solution

Added proper handling for all possible response types:

  • AuthLoginToken: normal case, returns new token
  • AuthLoginTokenSuccess: token already accepted, returns appropriate error
  • AuthLoginTokenMigrateTo: migration needed, returns MigrationNeededError

Changes

  • Modified Export method to use switch statement instead of simple type assertion
  • Added proper error handling for each response type
  • Maintains backward compatibility

This fix prevents runtime panics when AuthExportLoginToken returns unexpected (but valid) response types.

Copy link

codecov bot commented Jun 14, 2025

Codecov Report

Attention: Patch coverage is 42.10526% with 11 lines in your changes missing coverage. Please review.

Project coverage is 68.75%. Comparing base (6de998a) to head (a219bea).

Files with missing lines Patch % Lines
telegram/auth/qrlogin/qrlogin.go 42.10% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1571      +/-   ##
==========================================
- Coverage   69.00%   68.75%   -0.25%     
==========================================
  Files         443      443              
  Lines       21530    21545      +15     
==========================================
- Hits        14856    14813      -43     
- Misses       5740     5787      +47     
- Partials      934      945      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fr0ster added 5 commits June 15, 2025 18:23
…sponse types

The Export method now properly handles all possible response types from AuthExportLoginToken:
- AuthLoginToken: normal case, returns new token
- AuthLoginTokenSuccess: token already accepted, returns error
- AuthLoginTokenMigrateTo: migration needed, returns MigrationNeededError

Previously only AuthLoginToken was handled, which could cause unexpected type errors.
AuthLoginTokenSuccess indicates successful authentication, not an error.
Return empty token instead of error when login token was already accepted.
- Add tests for MigrationNeededError.Error method (0% -> 100%)
- Add tests for OnLoginToken function (0% -> 100%)
- Add tests for Token.Image method (0% -> 75%)
- Add tests for Import method with migration scenarios (43.5% -> 82.6%)
- Overall coverage improved from 67.0% to 84.6%
@fr0ster
Copy link
Author

fr0ster commented Jun 16, 2025

📝 Changes Made

✅ Fixed Commit Messages

Fixed all commit messages to follow Conventional Commits standard as required by commitlint:

  • Changed qrlogin: fix ...fix(qrlogin): ...
  • Changed feat(vscode): ...feat(qrlogin): ... (corrected scope)

📊 Significantly Improved Test Coverage

Added comprehensive tests to address codecov feedback:

Overall coverage: 67.0%84.6% (+17.6%)

Method-specific improvements:

  • MigrationNeededError.Error(): 0%100%
  • OnLoginToken(): 0%100%
  • Token.Image(): 0%75%
  • Import(): 43.5%82.6%

🧪 New Tests Added

  • TestMigrationNeededError_Error - Tests error message formatting
  • TestOnLoginToken - Tests login token handler setup and channel communication
  • TestToken_Image - Tests QR code image generation with different quality levels
  • TestQR_Import_WithMigration - Tests Import method with successful migration
  • TestQR_Import_MigrationError - Tests Import method when migration fails

🎯 Coverage Issues Addressed

This PR addresses the codecov report showing 42.10% patch coverage with 11 missing lines. The new tests specifically target the previously uncovered code paths in:

  • Error handling methods
  • QR code generation
  • Migration scenarios
  • Login token event handling

The commitlint check should now pass ✅ and codecov coverage should be significantly improved 📈

fr0ster added 2 commits June 17, 2025 11:15
…eived

When Export returns empty token (AuthLoginTokenSuccess case), Auth method
should wait for the loggedIn signal before calling Import, ensuring proper
synchronization with the login flow and fixing test failures on macOS Go 1.23.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant