Skip to content

Conversation

@ArangoGutierrez
Copy link
Collaborator

No description provided.

Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
@ArangoGutierrez ArangoGutierrez self-assigned this May 29, 2025
@ArangoGutierrez ArangoGutierrez merged commit 50c2875 into release-0.2 May 29, 2025
15 of 16 checks passed
Comment on lines +74 to +77
instance, err := manager.GetInstance(instanceID)
if err != nil {
return fmt.Errorf("failed to get instance %s: %v", instanceID, err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Problem: The code retrieves an instance but doesn't check if it exists before attempting to delete it. The error returned might be due to the instance not existing, which should be handled differently than other errors.

Suggested Change: Add specific handling for the case where the instance doesn't exist to provide better error messaging and avoid confusion.

Severity (1 - 4): 3 - MAJOR

Lines: 74-77

Suggested change
instance, err := manager.GetInstance(instanceID)
if err != nil {
return fmt.Errorf("failed to get instance %s: %v", instanceID, err)
}
instance, err := manager.GetInstance(instanceID)
if err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("instance %s does not exist", instanceID)
}
return fmt.Errorf("failed to get instance %s: %v", instanceID, err)
}

Generated by Claude 3.5 Sonnet

Was this helpful? 👍 👎

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.

3 participants