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

add asyncio.CancelledError to serialize_decorator #1015

Closed
wants to merge 3 commits into from

Conversation

geohotstan
Copy link

@geohotstan geohotstan commented Mar 15, 2024

while investigating issue #1013 I tried ctrl + c to start serializing for a deserialize run, but Team.serialize was not being triggered.

the traceback looks something like

... 
  File "/Users/zibo/fun/MetaGPT/.venv/lib/python3.11/site-packages/grpc/aio/_call.py", line 370, in _read                                                                            
    raw_response = await self._cython_call.receive_serialized_message()                                                                                                              
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                              
  File "src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi", line 372, in receive_serialized_message                                                                            
  File "src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi", line 126, in _receive_message                                                                           
  File "src/python/grpcio/grpc/_cython/_cygrpc/aio/callback_common.pyx.pxi", line 99, in execute_batch                                                                               
asyncio.exceptions.CancelledError                                                                                                                                                    
                                                                                                                                                                                     
During handling of the above exception, another exception occurred:                                                                                                                  
                                                                                                                                                                                     
Traceback (most recent call last):                                                                                                                                                   
  File "/Users/zibo/fun/MetaGPT/test.py", line 29, in <module>                                                                                                                       
    asyncio.run(startup("write number guessing game lol but make sure to cause error"))                                                                                              
  File "/opt/homebrew/Cellar/python@3.11/3.11.6/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/runners.py", line 190, in run                                       
    return runner.run(main)                                                                                                                                                          
           ^^^^^^^^^^^^^^^^                                                                                                                                                          
  File "/opt/homebrew/Cellar/python@3.11/3.11.6/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/runners.py", line 123, in run                                       
    raise KeyboardInterrupt()                                                                                                                                                        
KeyboardInterrupt                                  

where the KeyboardInterrupt was triggered inside asyncio.exceptions.CancelledError.
asyncio.exceptions.CancelledError was also not being caught by Exception because it inherits from BaseException
so ctrl + c ends the program without serializing.

not sure if this is intended or not since I'm very new to this repo

for the fix, I kept KeyboardInterrupt in in case non-async code gets added.

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.77%. Comparing base (ba1866f) to head (0fd8602).
Report is 7 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1015   +/-   ##
=======================================
  Coverage   82.76%   82.77%           
=======================================
  Files         224      224           
  Lines       13128    13129    +1     
=======================================
+ Hits        10866    10867    +1     
  Misses       2262     2262           

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

@@ -623,7 +624,7 @@ async def wrapper(self, *args, **kwargs):
try:
result = await func(self, *args, **kwargs)
return result
except KeyboardInterrupt:
except (asyncio.CancelledError, KeyboardInterrupt):
Copy link
Owner

Choose a reason for hiding this comment

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

@better629 could you take a look at this?

@better629
Copy link
Collaborator

@geohotstan
check the pr and existed implement. And do the execution metagpt "write a 2048 game".

use code before pr, do the ctrl+c

    "Competitive Analysis": [
        "2048 Game A: Simple interface, lacks responsive features",
        "play2048.co: Beautiful and responsive UI with my best score shown",
        "2048game.com: Responsive UI with my best score shown, but many ads"
    ],
^C2024-03-19 15:43:47.036 | ERROR    | metagpt.utils.common:wrapper:650 - KeyboardInterrupt:  occurs, start to serialize the project
2024-03-19 15:43:47.060 | ERROR    | metagpt.utils.common:wrapper:639 - Exception occurs, start to serialize the project, exp:
Traceback (most recent call last):
  File "/Users/wangjinlin/work/demo-code/MetaGPT/metagpt/utils/common.py", line 648, in wrapper
    return await func(self, *args, **kwargs)
  File "/Users/wangjinlin/work/demo-code/MetaGPT/metagpt/roles/role.py", line 550, in run
    rsp = await self.react()
KeyboardInterrupt

During handling of the above exception, another exception occurred:

use the pr, do the ctrl+c

2024-03-19 15:47:02.084 | ERROR    | metagpt.utils.common:wrapper:651 - KeyboardInterrupt:  occurs, start to serialize the project
2024-03-19 15:47:02.110 | ERROR    | metagpt.utils.common:wrapper:640 - Exception occurs, start to serialize the project, exp:
Traceback (most recent call last):
  File "/Users/wangjinlin/opt/anaconda3/envs/metagpt_310/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Users/wangjinlin/opt/anaconda3/envs/metagpt_310/lib/python3.10/asyncio/base_events.py", line 636, in run_until_complete
    self.run_forever()
KeyboardInterrupt

During handling of the above exception, another exception occurred:

all of the both generate a file workspace/storage/team/team.json. So previous code can catch the KeyboardInterrupt.
And it seems that asyncio.CancelledError is inherited from Exception.

I seems that you wrote a test.py, Can you try metagpt "write a 2048 game" to validate KeyboardInterrupt.

@geohotstan
Copy link
Author

geohotstan commented Mar 19, 2024

here is the code from test.py
took it from the quickstart

import asyncio
from metagpt.roles import (
    Architect,
    Engineer,
    ProductManager,
    ProjectManager,
    QaEngineer,
)
from metagpt.team import Team

async def startup(idea: str):
    company = Team()
    company.hire(
        [
            ProductManager(),
            Architect(),
            ProjectManager(),
            Engineer(),
            QaEngineer(),
        ]
    )
    company.invest(investment=3.0)
    company.run_project(idea=idea)

    await company.run(n_round=5)

asyncio.run(startup("write number game for blackjack"))

maybe it's because my KeyboardInterrupt is being triggered inside asyncio.run()?
🤔

@better629
Copy link
Collaborator

@geohotstan maybe, you can ref to metagpt/software_company.py.
I will take a look of your py file.

@iorisa iorisa added bug-fix and removed bug-fix labels Mar 26, 2024
@geekan geekan closed this May 18, 2024
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.

None yet

5 participants