- Notifications
You must be signed in to change notification settings - Fork928
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
a147216
to20d2cf1
Compare20d2cf1
to77a3d19
Compareif 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.