Skip to content

handeling edge case with correct error mssg #56

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 7 commits into
base: main
Choose a base branch
from

Conversation

vishwajeet-hash
Copy link

@vishwajeet-hash vishwajeet-hash commented Feb 14, 2025

fixes #55
After multiple testing i found the error to be recreated for some scenarios ....mainly when a meeting link with the same session and host in which the bot has been removed ...
If the meeting with same link is started again then again it will work.
In future this case also can be added where rejoining is also possible.

Copy link
Collaborator

@noah-duncan noah-duncan left a comment

Choose a reason for hiding this comment

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

Hey, this needed a few changes to make the tests pass. Did you ever run the tests? You can do so locally by running DJANGO_SETTINGS_MODULE=attendee.settings.test python manage.py test --keepdb --verbosity=2 bots.tests.test_bot_join_meeting.TestBotJoinMeeting.test_bot_in_fatal_error_does_not_create_meeting_ended_event

I needed to change the test case to this

    @patch('deepgram.DeepgramClient')
    @patch('bots.models.BotEventManager.create_event')
    @patch('bots.zoom_bot_adapter.zoom_bot_adapter.zoom', new_callable=create_mock_zoom_sdk)
    @patch('bots.zoom_bot_adapter.zoom_bot_adapter.jwt')
    def test_bot_in_fatal_error_does_not_create_meeting_ended_event(self, mock_create_event, MockDeepgramClient, mock_zoom_sdk_adapter,mock_jwt):
         # Setup mock objects and bot state
        bot = Bot.objects.create(state=BotStates.FATAL_ERROR, project=self.project)
        controller = BotController(bot.id)
        
        # Set up the necessary mocks for Deepgram, Zoom SDK, etc.
        MockDeepgramClient.return_value = create_mock_deepgram()
        
        # Create mock Zoom SDK instance
        mock_zoom_sdk_instance = create_mock_zoom_sdk()
        mock_zoom_sdk_adapter.return_value = mock_zoom_sdk_instance
        # Mock JWT token generation
        mock_jwt.encode.return_value = "fake_jwt_token"

        # Capture logs
        with self.assertLogs(level='INFO') as log:
        # Simulate meeting ended message
            message = {'message': BotAdapter.Messages.MEETING_ENDED}
            controller.bot_in_db = bot
            controller.individual_audio_input_manager = MagicMock()
            controller.closed_caption_manager = MagicMock()
            controller.cleanup = MagicMock()

            # Call the method that handles the message (take_action_based_on_message_from_adapter)
            controller.take_action_based_on_message_from_adapter(message)

            # Assert no event was created for MEETING_ENDED
            mock_create_event.assert_not_called()

            # Check if the log contains the expected message
            self.assertIn('INFO:root:Bot is in FATAL_ERROR state. Bot cannot rejoin once removed. End the meet for all and start meet again with same link', log.output)

            # Verify that cleanup was called
            controller.cleanup.assert_called_once()

            # Ensure the bot's state remains as FATAL_ERROR
            bot.refresh_from_db()
            self.assertEqual(bot.state, BotStates.FATAL_ERROR)

        # Verify expected SDK calls to ensure no meeting creation or interaction was attempted
        mock_zoom_sdk_instance.InitSDK.assert_not_called()
        mock_zoom_sdk_instance.CreateMeetingService.assert_not_called()
        mock_zoom_sdk_instance.CreateAuthService.assert_not_called()

And instead of printing, I had to use the logger, to pass the assert in the test
logging.info("Bot is in FATAL_ERROR state. Bot cannot rejoin once removed. End the meet for all and start meet again with same link")

@@ -552,10 +553,14 @@ def take_action_based_on_message_from_adapter(self, message):
if message.get("message") == BotAdapter.Messages.MEETING_ENDED:
print("Received message that meeting ended")
self.flush_utterances()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Were you able to reproduce getting the thing to error out? I couldn't. Anyway, maybe you can have a more generic print message, because this can happen for many different reasons. IE "Message that meeting ended was ignored because bot was already in the FATAL_ERROR state"

@@ -552,10 +553,14 @@ def take_action_based_on_message_from_adapter(self, message):
if message.get("message") == BotAdapter.Messages.MEETING_ENDED:
print("Received message that meeting ended")
self.flush_utterances()
if self.bot_in_db.state == BotStates.LEAVING:
BotEventManager.create_event(bot=self.bot_in_db, event_type=BotEventTypes.BOT_LEFT_MEETING)
if self.bot_in_db.state != BotStates.FATAL_ERROR:
Copy link
Collaborator

@noah-duncan noah-duncan Feb 20, 2025

Choose a reason for hiding this comment

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

If we can avoid introducing another level of nesting, that'd be good. Can you just have it do

if self.bot_in_db.state == BotStates.FATAL_ERROR:
     <log message>
     return

Before we get to the other stuff?

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.

Handle edge case when the MEETING_ENDED message is received when Bot is already in the FATAL_ERROR state
2 participants