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

Tool Raise Error #2421

Closed
WebsheetPlugin opened this issue Apr 17, 2024 · 11 comments
Closed

Tool Raise Error #2421

WebsheetPlugin opened this issue Apr 17, 2024 · 11 comments
Assignees
Labels
0.2 Issues which are related to the pre 0.4 codebase logging related to logging issue needs-triage tool-usage suggestion and execution of function/tool call

Comments

@WebsheetPlugin
Copy link

Is your feature request related to a problem? Please describe.

A) Is there a possibility of raising the error I get from calling a tool? Curently it's writing only back the text, without the stack trace
This would allow me to faster develop the app.

B) Another way of solving the problem would be to log the stack trace.

Describe the solution you'd like

No response

Additional context

No response

@sonichi sonichi added logging related to logging issue tool-usage suggestion and execution of function/tool call labels Apr 18, 2024
@sonichi
Copy link
Contributor

sonichi commented Apr 18, 2024

@davorrunje have you observed a similar need?

@davorrunje
Copy link
Contributor

In general, outputting full stack traces is a security issue because it can leak internal secrets used by functions and not accessible to LLM. This is why we catch an exception and write a generic message by default. If users wish to override such behaviour, they can wrap the body of the function and write their own exception handling (outputting string formatted by exception handling code) and assume all risks associated with it. In my opinion, we should not do this automatically because we could easily leak secrets to the LLM and the LLM could easily leak further by calling other functions or executing code.

@davorrunje davorrunje self-assigned this Apr 18, 2024
@WebsheetPlugin
Copy link
Author

WebsheetPlugin commented Apr 19, 2024

There is some misunderstanding. Here are three points to consider:

  1. Normally, a raised error would stop the execution. So when developing the app it would be best, as it would not wait for the whole conversation to finish.

  2. When I proposed to log the stack in B, I did not want to feed it back into the messages. I just wrote we should log those. Because curently you can get very unmeaningful errors.

  3. Regarding try/catch proposal. Yes, it would work, but it would make more sense to try to catch it if you like to put it back into the message stream and not the other way around...

My reasoning:

  • Most caught errors anyway need some prefix so they make sense for gpt.
  • This way you can exclude errors that you don't want to catch/write. So basically to do what I proposed in A).
  • I don't think it's a natural way to catch errors this way. In other parts of the code, you also don't catch and return them into the message stream. So why here? - Normally, you should "get the error" and then think about how to handle it yourself.
  • You are loose error reporting

@ekzhu
Copy link
Collaborator

ekzhu commented Apr 20, 2024

@WebsheetPlugin do you think a "debug mode" for function/tool executor makes sense, in which: every exception is raised and the app just dies?

I tend to agree with Davor that adding the stack trace to the error message should be a conscious choice, we can certainly make it easier to do that by adding an option to show the stack trace (off by default).

@WebsheetPlugin
Copy link
Author

WebsheetPlugin commented Apr 20, 2024

Ok guys :) You don't get me. Ofc, I agree with Davor that we should not add the stack trace to the error message. This is not the point I tried to make.

Why are we adding an error message back to the stream by default? Can someone explain to me this logic? Is this normal?

I think all this is done, because of some old openAi example, but if you think about this logic, it's not natural.
As it is final and irreversible(Or how can I stop the application with an error now? As Autogen decides for me to catch/try and feed into the message stream).

If someone wants likes this behaviour to happen, then he should put it into try/catch himself. Got me?

@ekzhu, I just read it to the end. Yes, your idea would solve it! but in my opinion, it should be the default state!

@ekzhu
Copy link
Collaborator

ekzhu commented Apr 20, 2024

I get you. There are two things:

  1. Whether we raise the exception in tool/function to stop the application
  2. Whether we want to include stack trace to the agent's message.

I think we can have options for both of the above.

@WebsheetPlugin
Copy link
Author

  1. I vote for this for sure.
  2. Not sure this is needed.

@WebsheetPlugin
Copy link
Author

Hey @ekzhu , is there any update on this? Tool debugging is very hard without this feature...

@ekzhu
Copy link
Collaborator

ekzhu commented May 21, 2024

@WebsheetPlugin we haven't updated this. For (1) above, would you like to submit a PR?

@davorrunje
Copy link
Contributor

I am a bit late to the party, but here are my two cents on the problem:

  1. Logging of errors in tool can be easily added, although you can do it on the application level easily. I will create an issue and a PR for that.
  2. Stopping the application on an error in tool calling is not desirable in my opinion. LLMs make mistakes and sometimes use the wrong parameters to call a tool regardless of their signature and specification. There should be a meaningful message in such cases that would be propagated to the tool. I propose a new type of Exception to be raise which would be propagated to the LLM and all other exception would be transformed into a generic one without many explanations due to security reasons. Stack traces can contain secrets, API keys etc. and you do not want to have LLM access to it.

@rysweet rysweet added 0.2 Issues which are related to the pre 0.4 codebase needs-triage labels Oct 2, 2024
@fniedtner fniedtner removed the feature label Oct 24, 2024
@jackgerrits
Copy link
Member

Consider migrating to 0.4 to address this as you can much more easily customize required behavior that you have. For allowing a tool call to be fatal I have created a new issue specifically for that #5340

You find find the migration guide for 0.2 to 0.4 here

@jackgerrits jackgerrits closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.2 Issues which are related to the pre 0.4 codebase logging related to logging issue needs-triage tool-usage suggestion and execution of function/tool call
Projects
None yet
Development

No branches or pull requests

7 participants