From c392ae7afdb477cb74cf13ac0585e5437fd57ead Mon Sep 17 00:00:00 2001 From: heyitsaamir Date: Fri, 6 Mar 2026 11:25:55 -0800 Subject: [PATCH 1/3] fix: downgrade expected token exchange failure logs from error to info The SSO token exchange returns 400/404/412 as part of the normal sign-in flow to trigger the permission picker UI. These expected failures were logged at ERROR level, creating noise. Now only unexpected HTTP errors log at ERROR; expected failures log at INFO. Co-Authored-By: Claude Opus 4.6 --- packages/apps/src/microsoft_teams/apps/app_oauth.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/app_oauth.py b/packages/apps/src/microsoft_teams/apps/app_oauth.py index 2876d246..0e851c37 100644 --- a/packages/apps/src/microsoft_teams/apps/app_oauth.py +++ b/packages/apps/src/microsoft_teams/apps/app_oauth.py @@ -60,16 +60,16 @@ async def sign_in_token_exchange( self.event_emitter.emit("sign_in", SignInEvent(activity_ctx=ctx, token_response=token)) return None except Exception as e: - ctx.logger.error( - f"Error exchanging token for user {activity.from_.id} in " - f"conversation {activity.conversation.id}: {e}" - ) if isinstance(e, HTTPStatusError): status = e.response.status_code if status not in (404, 400, 412): + ctx.logger.error( + f"Error exchanging token for user {activity.from_.id} in " + f"conversation {activity.conversation.id}: {e}" + ) self.event_emitter.emit("error", ErrorEvent(error=e, context={"activity": activity})) return InvokeResponse(status=status or 500) - ctx.logger.warning( + ctx.logger.info( f"Unable to exchange token for user {activity.from_.id} in " f"conversation {activity.conversation.id}: {e}" ) From ffb41a9faef5d4fabdca4a2b781ca07babc115dc Mon Sep 17 00:00:00 2001 From: heyitsaamir Date: Fri, 6 Mar 2026 11:31:28 -0800 Subject: [PATCH 2/3] test: remove logger level assertions from token exchange tests Co-Authored-By: Claude Opus 4.6 --- packages/apps/tests/test_app_oauth.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/apps/tests/test_app_oauth.py b/packages/apps/tests/test_app_oauth.py index fa4981d8..5f9b3535 100644 --- a/packages/apps/tests/test_app_oauth.py +++ b/packages/apps/tests/test_app_oauth.py @@ -168,9 +168,6 @@ async def test_sign_in_token_exchange_http_error_404(self, oauth_handlers, mock_ # Verify no error event emitted for 404 oauth_handlers.event_emitter.emit.assert_not_called() - # Verify warning logged - mock_context.logger.warning.assert_called_once() - # Verify failure response assert isinstance(result, InvokeResponse) and isinstance(result.body, TokenExchangeInvokeResponse) assert result.status == 412 @@ -215,9 +212,6 @@ async def test_sign_in_token_exchange_generic_exception( result = await oauth_handlers.sign_in_token_exchange(mock_context) - # Verify warning logged - mock_context.logger.warning.assert_called_once() - # Verify failure response assert isinstance(result, InvokeResponse) and isinstance(result.body, TokenExchangeInvokeResponse) assert result.status == 412 From bdee0a3693089db52731829e2125fbc0623755d0 Mon Sep 17 00:00:00 2001 From: heyitsaamir Date: Fri, 6 Mar 2026 11:32:15 -0800 Subject: [PATCH 3/3] fix: use local log variable and keep error level for non-HTTP exceptions Address PR review comments: - Use `log` variable instead of `ctx.logger` for consistency - Keep error-level logging + error event for non-HTTPStatusError exceptions - Only downgrade to info for expected HTTP 400/404/412 Co-Authored-By: Claude Opus 4.6 --- .../apps/src/microsoft_teams/apps/app_oauth.py | 16 +++++++++++----- packages/apps/tests/test_app_oauth.py | 5 +++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/app_oauth.py b/packages/apps/src/microsoft_teams/apps/app_oauth.py index 0e851c37..535eadf3 100644 --- a/packages/apps/src/microsoft_teams/apps/app_oauth.py +++ b/packages/apps/src/microsoft_teams/apps/app_oauth.py @@ -63,16 +63,22 @@ async def sign_in_token_exchange( if isinstance(e, HTTPStatusError): status = e.response.status_code if status not in (404, 400, 412): - ctx.logger.error( + log.error( f"Error exchanging token for user {activity.from_.id} in " f"conversation {activity.conversation.id}: {e}" ) self.event_emitter.emit("error", ErrorEvent(error=e, context={"activity": activity})) return InvokeResponse(status=status or 500) - ctx.logger.info( - f"Unable to exchange token for user {activity.from_.id} in " - f"conversation {activity.conversation.id}: {e}" - ) + log.info( + f"Unable to exchange token for user {activity.from_.id} in " + f"conversation {activity.conversation.id}: {e}" + ) + else: + log.error( + f"Unable to exchange token for user {activity.from_.id} in " + f"conversation {activity.conversation.id}: {e}" + ) + self.event_emitter.emit("error", ErrorEvent(error=e, context={"activity": activity})) return InvokeResponse( status=412, body=TokenExchangeInvokeResponse( diff --git a/packages/apps/tests/test_app_oauth.py b/packages/apps/tests/test_app_oauth.py index 5f9b3535..62ef79e5 100644 --- a/packages/apps/tests/test_app_oauth.py +++ b/packages/apps/tests/test_app_oauth.py @@ -212,6 +212,11 @@ async def test_sign_in_token_exchange_generic_exception( result = await oauth_handlers.sign_in_token_exchange(mock_context) + # Verify error event emitted for non-HTTP exceptions + oauth_handlers.event_emitter.emit.assert_called_once_with( + "error", ErrorEvent(error=generic_error, context={"activity": token_exchange_activity}) + ) + # Verify failure response assert isinstance(result, InvokeResponse) and isinstance(result.body, TokenExchangeInvokeResponse) assert result.status == 412