feat: added constants for Error Types

[35C4n0r]

What does this PR do?

• added logic in Swagger2.php to parse errors from APISpecs -> definitions->x-appwrite.
• added Error enums in twig templates.

Related PRs and Issues

Closes #698

[stnguyen90]

Wow, this is amazing! Would you be able to add test cases as well? For reference, here's a PR with tests for Flutter/Dart.

[35C4n0r]

Wow, this is amazing! Would you be able to add test cases as well? For reference, here's a PR with tests for Flutter/Dart.

This is what the generate enums will look like (Python code):

class ErrorType(Enum):
    """
    General errors thrown by the mock controller used for testing.
    """
    GeneralMock = 'general_mock'

    """
    The request contains one or more invalid arguments. Please refer to the endpoint documentation.
    """
    GeneralArgumentInvalid = 'general_argument_invalid'

@stnguyen90 what kind of tests would we want for the generated enums,
also I think someone is working on adding the testcases here #680

[stnguyen90]

@35C4n0r

what kind of tests would we want for the generated enums,

tests comparing the enum to the string value like:

ErrorType.GeneralMock == 'general_mock`

This is exactly how the enums will be used by developers using the SDK.

also I think someone is working on adding the testcases here #680

Ya but they wouldn't be able to add tests for this because the enum code is in your PR.

[mvarendorff]

Heyyo :) Guy working on the tests here! I am only just getting started, and the tests are a bit of a pain to set up from the ground up. So if this is merged, I don't mind rebasing my stuff and potentially including them.

Only thing that would help me would be a reference testcase or two in the dart / flutter SDK so I can port that to the other SDKs. I am currently on the Android front still and it's going to be some time until I get my hands on other SDKs.

Just offering this up because adding testcases across all SDKs is otherwise going to take a bit and probably cause more chaos merging things than if this was merged and I just included the tests in my work.

Thoughts @stnguyen90 ?

[35C4n0r]

@stnguyen90 This is what I came up with, 061fea8, if it looks good i'll add for the rest of the sdks.

[stnguyen90]

@geisterfurz007, @35C4n0r can add these test cases since they should be self contained and shouldn't impact the other tests too much.

[stnguyen90]

@35C4n0r, btw, best practices for commit messages is to use imperative (i.e. use add rather than added). For more info, see https://cbea.ms/git-commit/

[35C4n0r]

@geisterfurz007 what testing library are you planning to use for node runtime.
For now I'm using assert.strictEqual(), but let me know.

[mvarendorff]

@35C4n0r Whatever works for you for now is good, I think. So far I am only done-ish with Kotlin where I used kotlin-test / JUnit. Everything else is still pending for me. Should I pick another library anywhere for any good reason, I'd probably just migrate these tests over.

[35C4n0r]

@stnguyen90 I've added the Tests, PR is up for review.

[stnguyen90]

Great PR! 🤯 We left some comments during the review, please check them out.

[35C4n0r]

@stnguyen90 I have made changes according to the comments. Kindly re-review

[lohanidamodar]

@35C4n0r Thank you for update to the PR however, I'm still seeing so many tests fail, will you be able to investigate why?

[35C4n0r]

@lohanidamodar @stnguyen90 There were a minor issues that i've fixed.

This is what i've added in my spec to test in the local.

    "appwriteException": {
      "properties": {
        "message": {
          "type": "string",
          "description": "Error message.",
          "x-example": "Invalid id: Parameter must be a valid number"
        },
        "type": {
          "type": "string",
          "description": "Error type.",
          "enum": [
            "general_mock",
            "general_argument_invalid"
          ],
          "x-example": "argument_invalid"
        },
        "code": {
          "type": "integer",
          "description": "Error code.",
          "x-example": 400,
          "format": "int32"
        }
      },
      "x-appwrite": {
        "types": [
          {
            "code": 400,
            "type": "general_mock",
            "message": "General errors thrown by the mock controller used for testing."
          },
          {
            "code": 400,
            "type": "general_argument_invalid",
            "message": "The request contains one or more invalid arguments. Please refer to the endpoint documentation."
          }
        ]
      }

Now, the remaining tests for Python, Dart and Java are failing, cause the spec passed to the SDK doesn't have any error types (under x-appwrite) present in them, in which case no enum element is generated, what would be a better way to handle this would be to not generate the ErrorType enum if there are 0 elements. Also this issue is arising as the spec is not updated with the error types.

[35C4n0r]

@lohanidamodar @stnguyen90 any thoughts on it?

[stnguyen90]

what would be a better way to handle this would be to not generate the ErrorType enum if there are 0 elements.

Ya, let's be defensive and handle the case so things don't break.

Also this issue is arising as the spec is not updated with the error types.

The specs used in the tests are here. They should probably be updated to include the x-appwrite stuff.

[35C4n0r]

@stnguyen90 I have changed the test Spec to this https://github.com/appwrite/sdk-generator/blob/c429aa0432aa70c3e987106b7e8bb42d12b4a903/tests/resources/spec.json

There is still an issue where AppwrwiteException Class is already written in the src/exceptions.* and is again bieng generated in the Models/. because of this

sdk-generator/tests/resources/spec.json

Line 1138 in c429aa0

"appwriteException": {

which is leading to ambiguos naming and import errors, see: https://github.com/appwrite/sdk-generator/actions/runs/6901109358/job/18775353731?pr=724#step:8:819, https://github.com/appwrite/sdk-generator/actions/runs/6901109358/job/18775352926?pr=724#step:8:248

How should we fix this.

[stnguyen90]

How should we fix this?

Can we have an exclude to exclude creating models for things that end in Exception?

[35C4n0r]

@stnguyen90 @lohanidamodar all tests pass, the PR is ready for review.

[lohanidamodar]

@35C4n0r Awesome work, will work to get this merged as soon as possible.

[gewenyu99]

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

[35C4n0r]

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

@gewenyu99, Here is my DiscordID: huntersoulz

[gewenyu99]

Reaching out soon!