Skip to content
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

Report error responses as JSON #90

Merged
merged 17 commits into from
Mar 5, 2025
Merged

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Feb 27, 2025

For element-hq/element-meta#2732

This changes the following:

  • Errors reported by the application now respond with a JSON body containing both an error describing the problem, and an errcode which is machine readable.
  • Admins may now (optionally) specify a errcode for each rejection condition.

There is a docs/api.md to describe the change.

I have done this in a way that works, but I have no doubt this is horrendous Go code. Please be brutal.

@Half-Shot
Copy link
Contributor Author

Clientside implementation is in element-hq/element-web#29378

@Half-Shot Half-Shot requested a review from richvdh March 3, 2025 13:40
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

func (s *submitServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
// if we attempt to return a response without reading the request body,
// apache gets upset and returns a 500. Let's try this.
defer req.Body.Close()
defer io.Copy(ioutil.Discard, req.Body)

if req.Method != "POST" && req.Method != "OPTIONS" {
writeError(w, 405, submitErrorResponse{"Method not allowed. Use POST.", ErrCodeMethodNotAllowed})
writeError(w, 405, submitErrorResponse{Error: "Method not allowed. Use POST.", ErrorCode: ErrCodeMethodNotAllowed})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have just done

writeError(w, 405, submitErrorResponse{"Method not allowed. Use POST.", ErrCodeMethodNotAllowed, ""})

but this is fine

@richvdh richvdh merged commit a65f5e6 into main Mar 5, 2025
5 checks passed
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.

2 participants