- Notifications
You must be signed in to change notification settings - Fork927
fix(agent/agentcontainers): prevent reassigning proc.agent until successful#18609
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(agent/agentcontainers): prevent reassigning proc.agent until successful#18609
Conversation
a147216 to20d2cf1Compare20d2cf1 to77a3d19Compare| if err != nil { | ||
| returnagent, err | ||
| returnSubAgent{}, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Review: In either this or above, we could have returned a semi-valid agent and assigned toproc.agent was is not ideal.
| defer func() { | ||
| if err != nil { | ||
| // Best effort. | ||
| _, _ = a.api.DeleteSubAgent(ctx, &agentproto.DeleteSubAgentRequest{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The steps below should never fail, but if they do, we try to clean up.
4f44dd0 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This change applies a bit more defensive coding to the SubAgentClient. I don't have any concrete evidence of this going wrong other than some random observations of "duplicate agent", which are likely unrelated but prompted me to explore.