Skip to content
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

[BUG] - promptflow executor raises KeyError in _update_operation_context #2237

Closed
FriederikeBrezing opened this issue Mar 6, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@FriederikeBrezing
Copy link

Describe the bug
When running a promptflow which calls a second promptflow as a function, we occassionally (but not always) get the following error message:

`2024-03-05 10:34:51 +0000 18 execution.flow INFO Node service_caller completes.
[2024-03-05 10:34:53,913][pfserving-app][ERROR] - Promptflow serving app error: {'message': "'flow_id'", 'messageFormat': '', 'messageParameters': {}, 'code': 'SystemError', 'innerError': {'code': 'KeyError', 'innerError': None}}
[2024-03-05 10:34:53,914][pfserving-app][ERROR] - Promptflow serving error traceback: Traceback (most recent call last):
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/flask/app.py", line 1484, in full_dispatch_request
rv = self.dispatch_request()
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/flask/app.py", line 1469, in dispatch_request
return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/promptflow/_sdk/_serving/app.py", line 148, in score
flow_result = app.flow_invoker.invoke(data, run_id=run_id, disable_input_output_logging=disable_data_logging)
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/promptflow/_sdk/_serving/flow_invoker.py", line 175, in invoke
result = self._invoke(data, run_id=run_id, disable_input_output_logging=disable_input_output_logging)
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/promptflow/_sdk/_serving/flow_invoker.py", line 160, in _invoke
result = self.executor.exec_line(data, index=0, run_id=run_id, allow_generator_output=self.streaming())
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/promptflow/executor/flow_executor.py", line 714, in exec_line
line_result = self._exec_with_trace(
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/contextlib.py", line 126, in exit
next(self.gen)
File "/azureml-envs/prompt-flow/runtime/lib/python3.9/site-packages/promptflow/executor/flow_executor.py", line 747, in _update_operation_context
operation_context.pop(k)
KeyError: 'flow_id'


**How To Reproduce the bug**

Steps to reproduce the behavior, how frequent can you experience the bug:
1. Create a promptflow which invokes a second flows like this

second_flow = promptflow.load_flow(flow_path)
second_flow()

2. Invoke the (first) promptflow. 

**Expected behavior**
The flow should succeed.


**Running Information(please complete the following information):**
 - Promptflow Package Version: 16.0
 - Operating System: macos
 - Python Version using 3.9.18

**Additional context**
The error only occurs intermittently. We suspect that it might be a concurrency issue in `_update_operation_context`. 

The error does not occur with promptflow 14.0.6
@FriederikeBrezing FriederikeBrezing added the bug Something isn't working label Mar 6, 2024
@thy09
Copy link
Contributor

thy09 commented Mar 7, 2024

Thank you for reporting this bug, we will investigate it and fix it asap.

thy09 added a commit that referenced this issue Mar 8, 2024
# Description

Currently call a flow in a flow node will fail due to wrongly handled
operation context, in this PR, we fix it by copy the original context
and recover.
Issue: #2237

This pull request primarily focuses on refactoring the handling of
operation contexts in the `promptflow` package. The most significant
changes include the addition of a `copy` method to the
`OperationContext` class, the introduction of a `set_instance` class
method to the same class, and changes to the `exec_line_async` and
`_update_operation_context` methods in the `FlowExecutor` class to
utilize these new methods.

Here is a breakdown of the key changes:

Changes to `OperationContext` class in
`src/promptflow/promptflow/_core/operation_context.py`:

* A `copy` method has been added to the `OperationContext` class, which
creates a copy of the current operation context and returns it. This
method also ensures that a copy of the `_OTEL_ATTRIBUTES` is made.
* A `set_instance` class method has been introduced. This method allows
setting a new instance of the operation context.

Changes to `FlowExecutor` class in
`src/promptflow/promptflow/executor/flow_executor.py`:

* In the `exec_line_async` method, the `OperationContext`'s new `copy`
method is used to create a copy of the original operation context.
* The `_update_operation_context` method has been significantly
simplified. Instead of manually reverting changes to the operation
context, it now simply restores the original context using the
`OperationContext`'s `set_instance` method.

These changes simplify the code and make the handling of operation
contexts more robust and less error-prone.

# All Promptflow Contribution checklist:
- [ ] **The pull request does not introduce [breaking changes].**
- [ ] **CHANGELOG is updated for new features, bug fixes or other
significant changes.**
- [ ] **I have read the [contribution guidelines](../CONTRIBUTING.md).**
- [ ] **Create an issue and link to the pull request to get dedicated
review from promptflow team. Learn more: [suggested
workflow](../CONTRIBUTING.md#suggested-workflow).**

## General Guidelines and Best Practices
- [ ] Title of the pull request is clear and informative.
- [ ] There are a small number of commits, each of which have an
informative message. This means that previously merged commits do not
appear in the history of the PR. For more information on cleaning up the
commits in your PR, [see this
page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).

### Testing Guidelines
- [ ] Pull request includes test coverage for the included changes.

Co-authored-by: Heyi <heta@microsoft.com>
@thy09
Copy link
Contributor

thy09 commented Mar 13, 2024

The fix has been merged, will be released in the next version.

@thy09 thy09 closed this as completed Mar 13, 2024
@FriederikeBrezing
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants