Skip to content

[fix-2940][framework]: Add lazy caching of http codes for error handling and fix any broken Http error messages#2984

Merged
klesh merged 10 commits intoapache:mainfrom
merico-ai:issues/2940
Sep 15, 2022
Merged

[fix-2940][framework]: Add lazy caching of http codes for error handling and fix any broken Http error messages#2984
klesh merged 10 commits intoapache:mainfrom
merico-ai:issues/2940

Conversation

@keon94
Copy link
Contributor

@keon94 keon94 commented Sep 6, 2022

Summary

  1. Reworked the Type.Combine logic for combining multiple errors
  2. Made some improvements to the Error interface as far as getting message information about that error, and returning an extended error JSON to the front-end with the structure:
{
 "success": false,
 "message": "some string",
 "causes": ["cause1 string", "cause2 string", "cause3 string"]
}
  1. All Http codes now supported, not just the statically provided Types (fixes [Bug][Core] Database Migration Required uses Wrong Status Code 500  #2940)
  2. The work from (1) fixes the problem in [Bug][Core] GitHub Plugin PAT Token TEST Degraded #3031

Does this close any open issues?

Closes #2940, #3031

Screenshots

Other Information

@keon94 keon94 force-pushed the issues/2940 branch 3 times, most recently from b88e0f4 to 1697a90 Compare September 7, 2022 20:38
@keon94 keon94 marked this pull request as ready for review September 8, 2022 04:52
@keon94 keon94 changed the title [fix-2940][framework]: Add lazy caching of http codes for error handling [fix-2940][framework]: Add lazy caching of http codes for error handling and fix any broken Http error messages Sep 9, 2022
@keon94 keon94 force-pushed the issues/2940 branch 3 times, most recently from ab04cb4 to 9d94c83 Compare September 9, 2022 17:12
@keon94
Copy link
Contributor Author

keon94 commented Sep 9, 2022

@klesh this is ready for review. I tuned things in accordance with the proposal here. cc @e2corporation

@keon94
Copy link
Contributor Author

keon94 commented Sep 12, 2022

Sample plugin execution failure (error message display fixed)
image

@keon94
Copy link
Contributor Author

keon94 commented Sep 13, 2022

Fix for #3031 screenshot:
image

@keon94
Copy link
Contributor Author

keon94 commented Sep 13, 2022

Fix for #3031 screenshot: image

error status is now 400 like before.

@klesh
Copy link
Contributor

klesh commented Sep 13, 2022

@keon94 Does it fix #2940? Could you provide screenshot for it?

@keon94
Copy link
Contributor Author

keon94 commented Sep 13, 2022

@klesh Yes.
image

@keon94 keon94 force-pushed the issues/2940 branch 3 times, most recently from ca240fa to df7c3be Compare September 14, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][Core] Database Migration Required uses Wrong Status Code 500

2 participants