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

Error in WaitForStateAsync is not wrapped #26

Closed
avorima opened this issue May 25, 2022 · 11 comments
Closed

Error in WaitForStateAsync is not wrapped #26

avorima opened this issue May 25, 2022 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@avorima
Copy link

avorima commented May 25, 2022

Description

The error in this line is neither wrapped nor even formatted correctly which means that it cannot be handled.

Additionally, the waitForStateWithChanel has a typo and the async wait for state or deletion code has a few issues. Code execution continues past writing of the done and state channels. For example here:

if err != nil {
	ch <- StateChannel{
		"",
		fmt.Errorf("error occured when calling the fn function", err),
	}
	done <- true
        // code execution should not go past this point
}

This pattern is found throughout the waitForStateWithChanel and waitForDeletionWithChannel functions. waitForDeletionWithChannel even has a risk of NPE because the apiResponse that is checked for nil here is then dereferenced a few lines after it. waitForStateWithChanel has the issue that it will hang when an error occurs. A message is sent to the done channel here and eventually another gets sent here. This means that this function will never exit since the exit condition done is checked on the next iteration and write to the done channel is blocked by it's size of 1.

This can be prevent by simply adding some continues or using an else block.

See: https://go.dev/play/p/0ENsyvFU4Bf

The code looks auto-generated so I don't know if I should contribute a patch. Please let me know.

@avorima avorima added the bug Something isn't working label May 25, 2022
@gabi-ionos
Copy link

Hello, thanks for noticing I will take a look into it.

@Ntr0
Copy link

Ntr0 commented Jun 17, 2022

Please note that the missing error wrapping affects all Wait* methods.

@gabi-ionos
Copy link

gabi-ionos commented Jun 22, 2022

can you be more specific, please? Are you referring to the fact that the error should be a specific type of error(like a waitForErr of type err) to figure out what type of error is and from where that error was thrown?

@gabi-ionos
Copy link

The bug was solved and the new implementation will be able in the next release.
If anybody want to contribute to the sdk go, I think is ok to open a pull request to the sdk-go, and I will add the code in the files that are generating the sdk.

@avorima
Copy link
Author

avorima commented Jul 6, 2022

@gabisavu-ionos The issue still isn't resolved in the current master. When will the next release land?

@gabi-ionos
Copy link

The release is not made yet, I'm not sure when the release will be made, but I can inform you through a comment here or a message on chat.

@avorima
Copy link
Author

avorima commented Jul 7, 2022

Could we keep this issue open until the fix is done?

@gabi-ionos
Copy link

yes, sure

@gabi-ionos gabi-ionos reopened this Jul 7, 2022
@avorima
Copy link
Author

avorima commented Jul 7, 2022

Thanks

@piepmatz
Copy link

piepmatz commented Aug 9, 2022

Can be closed as https://github.com/ionos-cloud/sdk-go/releases/tag/v6.1.2 is out, I guess.

@avorima
Copy link
Author

avorima commented Aug 9, 2022

The release also has issues but at least it addressed the ones mentioned here.

@avorima avorima closed this as completed Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants