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

Use Jason.encode! when sending errors #161

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

daskycodes
Copy link

@daskycodes daskycodes commented Apr 15, 2022

At the moment Twirp.Plug.send_error/2 uses Twirp.Encoder.encode/3 for encoding the error.

This leads to a problem where the defimpl Jason.Encoder for the Twirp.Error struct is not being used, and thus the :__exception__ field is being sent with every error response.

The twirp specification v7 describes how error responses must be encoded:

 {
  "code": "permission_denied",
  "msg": "Thou shall not pass",
  "meta": {
    "target": "Balrog",
    "power": "999"
  }
}

code and msg are required while meta is optional.

The twirp-elixir implementation will send the following:

{
  "__exception__": true,
  "code": "permission_denied",
  "msg": "Thou shall not pass",
  "meta": {
    "target": "Balrog",
    "power": "999"
  }
}

The twirp go implementation is very strict about the errors

var tj twerrJSON
dec := json.NewDecoder(bytes.NewReader(respBodyBytes))
dec.DisallowUnknownFields()
if err := dec.Decode(&tj); err != nil || tj.Code == "" {
	// Invalid JSON response; it must be an error from an intermediary.
	msg := fmt.Sprintf("Error from intermediary with HTTP status code %d %q", statusCode, statusText)
	return twirpErrorFromIntermediary(statusCode, msg, string(respBodyBytes))
}

This leads the Go implementation to handle the error as if it originated from an intermediary (e.g. a reverse proxy) and translating the HTTP status code 404 as a bad_route instead of the original error.

This PR makes use of Jason.encode!/1 in the Twirp.Plug.send_error/2 to fix this issue, as it will use the protocol implementation found in Twirp.Error.

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.

1 participant