[Bug][Core] GitHub Plugin PAT Token TEST Degraded

[e2corporation]

Search before asking

• [X] I had searched in the issues and found no similar issues.

What happened

GitHub connections are unable to complete a Test operation, API response returns Internal Server 500 instead of expected.

• The GitHub Provider is unable to complete a successful token test even though a valid token is being used.
• The "invalid" token message is not returned by API personal access tokens cannot be properly validated

{
  "success": false,
  "message": "error executing the requested resource for plugin github"
}

What you expected to happen

• If a Token Test is successful the API responds with expected true response { success: true ....}
• If a Token Test is unsuccessful the API responds with false response & applicable message {success: false, message: .... }

How to reproduce

Test a GitHub Data Connection

Anything else

No response

Version

main

Are you willing to submit PR?

• [ ] Yes I am willing to submit a PR!

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[e2corporation]

Hey what's up @keon94, this is the GitHub TEST Endpoint I issue we discussed earlier. Thanks.

[keon94]

Okay, I'll expand the scope of #2940 and push a fix there since it's already in progress. @e2corporation Does config-ui consume any other endpoints that would return a certain error JSON struct part of the response?

[keon94]

And as an idea, probably for another ticket, it might be a good idea to standardize the Backend->Frontend error-message format across all APIs to use some sort of JSON on the response. For instance something like:

{
 "message": "some string",
 "causes": ["cause1", "cause2", "cause3"]
}

[klesh]

@keon94 It was unified as the following

{
    "success": true|false,
    "message": "errors"
}

I think we should keep these 2 fields to maintain FE compatibility as well as other users that call our APIs directly, and add extra fields as you see fit.

[keon94]

@e2corporation, what do you think about this output?

For multiple invalid tokens:

[e2corporation]

@keon94 It will require updating a couple things on UI but I think that spec could work.

[keon94]

@e2corporation #2984 has been merged. Now the backend returns a JSON like the one I mentioned above, so if there's any UI side changes that are needed they can be started now.

[e2corporation]

@e2corporation #2984 has been merged. Now the backend returns a JSON like the one I mentioned above, so if there's any UI side changes that are needed they can be started now.

ok cool, after I get other core tasks finished I'll have to make some patches on UI to digest the new causes key where applicable.

[klesh]

Th problem seems to be fixed in main branch bf4de4df3bda079b4cca2ae9e97e8cc9e3d46c54 Close it for now