Skip to content

Commit

Permalink
Python: Properly handle exceptions in the kernel using the exception …
Browse files Browse the repository at this point in the history
…metadata (#5391)

### Motivation and Context

Two things: 1) Add the exception to the on function invoked args if an
exception exists in the function result. 2) Set the stepwise planner's
goal to the plan's description, not the name.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

The PR addresses:
- Properly handling exceptions to raise them from the kernel if a
function result contains an exception in the metadata. Closes #5385
- In the stepwise planner, the goal was getting set as the plan's name,
instead of the description. Closes #5316

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄
  • Loading branch information
moonbox3 authored Mar 8, 2024
1 parent c57608c commit bf5d21c
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 13 deletions.
6 changes: 4 additions & 2 deletions python/semantic_kernel/functions/kernel_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ async def invoke(
return await self._invoke_internal(kernel, arguments)
except Exception as exc:
logger.error(f"Error occurred while invoking function {self.name}: {exc}")
return FunctionResult(function=self.metadata, value=None, metadata={"error": exc, "arguments": arguments})
return FunctionResult(
function=self.metadata, value=None, metadata={"exception": exc, "arguments": arguments}
)

@abstractmethod
async def _invoke_internal_stream(
Expand Down Expand Up @@ -204,4 +206,4 @@ async def invoke_stream(
yield partial_result
except Exception as e:
logger.error(f"Error occurred while invoking function {self.name}: {e}")
yield FunctionResult(function=self.metadata, value=None, metadata={"error": e, "arguments": arguments})
yield FunctionResult(function=self.metadata, value=None, metadata={"exception": e, "arguments": arguments})
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ async def _handle_complete_chat_stream(
return # Exit after processing all iterations
except Exception as e:
logger.error(f"Error occurred while invoking function {self.name}: {e}")
yield FunctionResult(function=self.metadata, value=None, metadata={"error": e})
yield FunctionResult(function=self.metadata, value=None, metadata={"exception": e})

async def _handle_complete_text_stream(
self,
Expand All @@ -306,7 +306,7 @@ async def _handle_complete_text_stream(
return
except Exception as e:
logger.error(f"Error occurred while invoking function {self.name}: {e}")
yield FunctionResult(function=self.metadata, value=None, metadata={"error": e})
yield FunctionResult(function=self.metadata, value=None, metadata={"exception": e})

def add_default_values(self, arguments: "KernelArguments") -> KernelArguments:
"""Gathers the function parameters from the arguments."""
Expand Down
2 changes: 1 addition & 1 deletion python/semantic_kernel/kernel.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ def on_function_invoked(
kernel_function_metadata=kernel_function_metadata,
arguments=arguments,
function_result=function_result,
exception=exception,
exception=exception or function_result.metadata.get("exception", None) if function_result else None,
)
if self.function_invoked_handlers:
for handler in self.function_invoked_handlers.values():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def create_plan(self, goal: str) -> Plan:
plan_step._outputs.append("plugin_count")
plan_step._outputs.append("steps_taken")

plan = Plan(goal)
plan = Plan(description=goal)

plan.add_steps([plan_step])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ async def create_with_data_chat_function(get_aoai_config, kernel: Kernel, create


@pytest.mark.asyncio
@pytest.mark.xfail(reason="The test is failing a 400 saying the request body is invalid. Will investigate.")
@pytestmark
async def test_azure_e2e_chat_completion_with_extensions(
create_with_data_chat_function,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def non_async_function() -> str:
assert result.value == ""

async for partial_result in native_function.invoke_stream(kernel=None, arguments=None):
assert isinstance(partial_result.metadata["error"], NotImplementedError)
assert isinstance(partial_result.metadata["exception"], NotImplementedError)


@pytest.mark.asyncio
Expand All @@ -157,7 +157,7 @@ async def async_function() -> str:
assert result.value == ""

async for partial_result in native_function.invoke_stream(kernel=None, arguments=None):
assert isinstance(partial_result.metadata["error"], NotImplementedError)
assert isinstance(partial_result.metadata["exception"], NotImplementedError)


@pytest.mark.asyncio
Expand Down Expand Up @@ -227,7 +227,7 @@ def my_function(input: str) -> str:
func = KernelFunction.from_method(my_function, "test")

result = await func.invoke(kernel=None, arguments=KernelArguments())
assert isinstance(result.metadata["error"], FunctionExecutionException)
assert isinstance(result.metadata["exception"], FunctionExecutionException)


@pytest.mark.asyncio
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ async def test_invoke_exception():
) as mock:
mock.return_value = [ChatMessageContent(role="assistant", content="test", metadata={})]
result = await function.invoke(kernel=kernel)
assert isinstance(result.metadata["error"], Exception)
assert isinstance(result.metadata["exception"], Exception)

with patch(
"semantic_kernel.connectors.ai.open_ai.services.open_ai_chat_completion.OpenAIChatCompletion.complete_chat_stream",
Expand All @@ -193,7 +193,7 @@ async def test_invoke_exception():
StreamingChatMessageContent(choice_index=0, role="assistant", content="test", metadata={})
]
async for result in function.invoke_stream(kernel=kernel):
assert isinstance(result.metadata["error"], Exception)
assert isinstance(result.metadata["exception"], Exception)


@pytest.mark.asyncio
Expand Down Expand Up @@ -237,15 +237,15 @@ async def test_invoke_exception_text():
) as mock:
mock.return_value = [TextContent(text="test", metadata={})]
result = await function.invoke(kernel=kernel)
assert isinstance(result.metadata["error"], Exception)
assert isinstance(result.metadata["exception"], Exception)

with patch(
"semantic_kernel.connectors.ai.open_ai.services.open_ai_text_completion.OpenAITextCompletion.complete_stream",
side_effect=Exception,
) as mock:
mock.__iter__.return_value = []
async for result in function.invoke_stream(kernel=kernel):
assert isinstance(result.metadata["error"], Exception)
assert isinstance(result.metadata["exception"], Exception)


@pytest.mark.asyncio
Expand Down

0 comments on commit bf5d21c

Please sign in to comment.