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

Return readable errors on invalid input instead of crashing on array indices #2091

Open
mark-at-nuna opened this issue Oct 3, 2023 · 3 comments
Labels
category:other feature-request Used to mark issues with provider's missing functionalities

Comments

@mark-at-nuna
Copy link

Is your feature request related to a problem? Please describe.

I recently tried to create a masking policy application resource without passing in a fully qualified name for the table. The error message I received, amid a stack trace, was panic: runtime error: index out of range [1] with length 1 .

After digging through the stack trace and finding this line it was obvious that I needed at least two periods in my input. However, that required me to be familiar with both Golang and Snowflake syntax.

It would be much better to receive an error message explaining what I did wrong, e.g. "resource must be fully qualified, in the form <DATABASE_NAME>.<SCHEMA_NAME>.<TABLE_NAME>".

As to whether that error should propagate through a panic statement or up through the stack as a Golang error, I don't have a strong opinion.

@mark-at-nuna mark-at-nuna added the feature-request Used to mark issues with provider's missing functionalities label Oct 3, 2023
@sfc-gh-asawicki
Copy link
Collaborator

Hey @mark-at-nuna.

Thanks for bringing that to our attention. We are also unhappy with how errors are propagated to the end user. We have it on our roadmap. We will share the details as soon as we have them.

@sfc-gh-jcieslak
Copy link
Collaborator

Hey @mark-at-nuna
We are currently working on improving identifiers and readable error messages are one of the main things we want to focus on. Although, right now the new error messages are not yet present, during resources preparation for v1 we will be applying our new identifier conventions that should help with debugging related issues. Here's an example from the snowflake_grant_privileges_to_account_role, the errors for incorrect identifier form should look similarly to this:

│ Error: Invalid identifier type
│ 
│   with snowflake_grant_privileges_to_account_role.test,
│   on main.tf line 20, in resource "snowflake_grant_privileges_to_account_role" "test":
│   20:     object_name = "TEST_DATABASE.TEST_SCHEMA.TEST_TABLE.TOO_LONG_ID"
│ 
│ Expected SchemaObjectIdentifier identifier type, but got: sdk.TableColumnIdentifier. The correct form of the fully qualified name for this field is:
│ <database_name>.<schema_name>.<name>, but was <database_name>.<schema_name>.<table_name>.<column_name>

I'm guessing you refer to the masking policy that's present on our essential objects list, so the resource should be soon refactored.

@sfc-gh-jmichalak
Copy link
Collaborator

Hi @mark-at-nuna 👋

Sorry for late response, but masking policies have been reworked in v0.96 (release, migration guide). Please update the provider.

We improved error handling in this resource, and as @sfc-gh-jcieslak stated, we're improving identifier parsing and handling in each resource we are reworking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:other feature-request Used to mark issues with provider's missing functionalities
Projects
None yet
Development

No branches or pull requests

4 participants