superfaceai / cli

Let AI connect the APIs for you
https://superface.ai
MIT License
22 stars 4 forks source link

[BUG] syntax error in the generated profile #348

Closed julpat closed 1 year ago

julpat commented 1 year ago

Expected Behavior

The generated profile will not contain syntax errors.

Current Behavior

From time to time generated profile contains syntax errors. These files are stored in DB and listed in My Comlinks at superface.ai/app. When a user is trying to see the detail of the Comlink it ends with a 500 error on trying to parse that file.

Possible Solution

Do not store generated profiles with syntax errors eventually add syntax check as part of generating process and do not allow it to continue if the file is broken -> generate profiles only without syntax errors.

Steps to Reproduce

Did not find steps to reproduce this every time, but one case is when the profile contains enum definition as in the attached profile for a pipedrive provider with prompt: create organization

name = "organizations-management/add-organization"
version = "0.0.0"

"Adds a new organization with specified details, including custom fields."
usecase CreateOrganization unsafe  {
    input {
        "The name of the organization."
        name! string!

        "The optional creation date & time of the organization in UTC. Requires admin user API token. Format: YYYY-MM-DD HH:MM:SS"
        add_time string!

        "The ID of the user who will be marked as the owner of this organization. When omitted, the authorized user ID will be used."
        owner_id number!

        "The visibility of the organization. If omitted, the visibility will be set to the default visibility setting of this item type for the authorized user."
        visible_to enum {
            "The visibility of the organization. If omitted, the visibility will be set to the default visibility setting of this item type for the authorized user."
            1

            "The visibility of the organization. If omitted, the visibility will be set to the default visibility setting of this item type for the authorized user."
            3

            "The visibility of the organization. If omitted, the visibility will be set to the default visibility setting of this item type for the authorized user."
            5

            "The visibility of the organization. If omitted, the visibility will be set to the default visibility setting of this item type for the authorized user."
            7
        }!
    }

    result {
        success! boolean!

        data! {
            id! number!

            name! string!

            owner_id! number!

            add_time! string!

            visible_to! enum {
                1

                3

                5

                7
            }!
        }!
    }!

    error {
        "Indicates if the response is successful or not."
        success! boolean!

        "The error message."
        error! string!
    }!

    example InputExample {
        input {
            name = 'Acme Corporation',
            add_time = '2022-11-01T08:55:59Z',
            owner_id = 1,
            visible_to = '1',
        }
        result {
            success = true,
            data = {
                name = 'placeholder',
                add_time = 'placeholder',
                visible_to = 'placeholder',
            },
        }
    }
}
Jakub-Vacek commented 1 year ago

When did you test this? Numeric enums issue should be already resolved: https://github.com/superfaceai/cli/issues/324.

julpat commented 1 year ago

Yes, it's a bit older (19.7.), I got into this because of 500 errors in Air/Brain. Enums are fixed, but are we sure, that only valid profiles will be generated/stored? Is there some check in the authoring process?

Jakub-Vacek commented 1 year ago

We have checks on multiple levels - in Engine each AST node is checked for it's kind and whole AST is checked with assert (which validates it agains schema). On the CLI side we compile profile source to AST - validate it once again.

So as long as Schema is correct we are good, I plan to revisit this as soon as we have new Parser (see my question in Slack).

julpat commented 1 year ago

Ok, thanks for the confirmation. Let's consider this resolved. I will add a fix to Air to not fail with 500 errors for already created Comlinks with syntax a error.