-
Notifications
You must be signed in to change notification settings - Fork 3
feat() Map domain errors to gRPC status codes #1077
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
base: main
Are you sure you want to change the base?
Conversation
… also add check if challenge has internal numeric ID when created manually with the API, as CTFd expects numeric IDs
|
Why wanting to enforce numeric IDs ? |
when CM had string IDs (created them directly via the CM API during testing), the plugin tried to link to /admin/challenges/ and blew up because CTFd challenge IDs are numeric PKs. But if you want CM to be agnostic to CTFd I can remove that check again. The CM plugin PR already prevents the admin page from crashing even if string IDs exist in CM. |
|
Yeah I would prefer to stick with strings for IDs here 👍 |
done! 😄 |
pandatix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good, did not run it yet. There are some improvements to be made first then it will be tested more thoroughly 😉
|
I have added the requested changes, for error traces I will do a new PR if I get to it :) |
Enforce numeric challenge IDs (CTFd compatibility).Why: To stop the gRPC gateway from emitting generic 500s and give downstream (CTFd plugin/UI) actionable error codes/messages. See: ctfer-io/ctfd-chall-manager#238
Issues: Partially fixes ctfer-io/ctfd-chall-manager#232; addresses ctfer-io/ctfd-chall-manager#225.
I accept the code of conduct.