docs: clarify abort-on-function-call example#1843
docs: clarify abort-on-function-call example#1843Rohan5commit wants to merge 1 commit intoopenai:masterfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc00784557
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const finalFunctionCall = await runner.finalFunctionCall(); | ||
| console.log('Final function call:', finalFunctionCall); | ||
| await runner.done().catch(() => {}); |
There was a problem hiding this comment.
Handle only abort errors in
done() catch
This line suppresses all failures from runner.done(), not just the expected APIUserAbortError from runner.abort(). In the example, network/auth/API errors would be silently ignored and execution would continue as if the run ended normally, which can mislead users copying this pattern. Catching and filtering only abort errors (and rethrowing others) would keep the abort example accurate without hiding real failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Updates helpers.md to clarify how aborting a runTools() flow behaves, specifically that calling runner.abort() ends the run and causes final* helpers to reject, and adjusts the “Abort on a function call” example accordingly.
Changes:
- Refines the narrative around aborting a runner and the consequences for
final*helpers. - Updates the example to capture the terminating tool invocation inside the tool function and wait on
runner.done(). - Adds an
abortevent handler in the example output.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const finalFunctionCall = await runner.finalFunctionCall(); | ||
| console.log('Final function call:', finalFunctionCall); | ||
| await runner.done().catch(() => {}); |
There was a problem hiding this comment.
await runner.done().catch(() => {}); swallows all errors (network/API errors, parsing errors, etc.), not just the expected abort. This can lead readers to silently ignore failures. Catch the error and only ignore APIUserAbortError (or check error.name === 'APIUserAbortError'), and rethrow anything else.
| await runner.done().catch(() => {}); | |
| try { | |
| await runner.done(); | |
| } catch (error) { | |
| if ((error as { name?: string })?.name !== 'APIUserAbortError') { | |
| throw error; | |
| } | |
| } |
| function: function updateDatabase(props, runner) { | ||
| runner.abort() | ||
| terminatingCall = props; | ||
| runner.abort(); | ||
| }, |
There was a problem hiding this comment.
In this example terminatingCall is assigned props, which are the tool/function arguments, not the full function/tool call (name/id/raw arguments). The variable name and log message (“Function call that ended the run”) are misleading; consider renaming to reflect that it’s arguments, or capture the actual tool call object via the appropriate event and log that instead.
Changes being requested
Clarify the
Abort on a function callexample so it no longer callsfinalFunctionCall()afterrunner.abort(), and document that thefinal*helpers reject once the run has been aborted.Additional context & links
Abort on a function callexample is confusing #996helpers.mdgit diff --check