fix: Return correct http error codes instead of 500. Fixes #9237

[isubasinghe]

Signed-off-by: Isitha Subasinghe isitha@pipekit.io

Fixes #9237

Please do not open a pull request until you have checked ALL of these:

• Create the PR as draft .
• Run make pre-commit -B to fix codegen and lint problems.
• Sign-off your commits (otherwise the DCO check will fail).
• Use a conventional commit message (otherwise the commit message check will fail).
• "Fixes #" is in both the PR title (for release notes) and this description (to automatically link and close the issue).
• Add unit or e2e tests. Say how you tested your changes. If you changed the UI, attach screenshots.
• Github checks are green.
• Once required tests have passed, mark your PR "Ready for review".

If changes were requested, and you've made them, dismiss the review to get it reviewed again.

[alexec]

This does not look like the correct approach to me. There is an error translator somewhere you could take a look at.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

[sarabala1979]

@alexec Can you provide more information to @isubasinghe, So, he can refactor?

[isubasinghe]

@sarabala1979 this is a refactor from the old one which Alex commented on. Every error goes through ToStatusError.
Here I try to extract an ArgoError and map it into a grpc error, failing that I try to extract a k8s error and map that into a grpc error. It is inspired from the error handler in

argo-workflows/server/artifacts/artifact_server.go

Line 353 in eb2c54b

func (a *ArtifactServer) httpFromError(err error, w http.ResponseWriter) {

and this: https://github.com/grpc-ecosystem/grpc-gateway/blob/8952e38d5addd28308e29c272c696a578aa8ace8/runtime/errors.go#L36

In a general sense, the most relevant file here is just: https://github.com/argoproj/argo-workflows/pull/9916/files#diff-6ec3cdfd853ed9734ca0ba5e09bfc02c4a2ff21478a1d264ce3e4277e40f4c69

The others are just calling this.

[caelan-io]

What are your thoughts @alexec ? Can we merge to close #9237?

[JPZ13]

Code looks great. Running well when tested locally. One quick nit, can you change the PR title to be something like fix: Return correct http error codes. Fixes #9237 or something similar?

[alexec]

This looks great. I think maybe a couple more tests?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

[stale]

This issue has been closed due to inactivity. Feel free to re-open if you still encounter this issue.

[JPZ13]

Might have to run make codegen again to get the docs test to pass. I think this is due for a merge soon

lack of bandwidth