service.twirp.go errorFromResponse function is broken when intermediary returns json

[timclark-bgch]

I am testing my server with invalid auth credentials through an AWS API Gateway.

The API Gateway returns 403 with the incorrect credentials with this JSON.

{"message":"Signature expired: 19700101T000000Z is now earlier than 20190612T110154Z (20190612T110654Z - 5 min.)"}

This results in the twirp client returning an empty twirp.Error.

This is because that JSON will unmarshal correctly into a twerrJSON struct with empty fields.

I suspect that better checking of the JSON is required in this fragment of generated code:

      var tj twerrJSON
    if err := json.Unmarshal(respBodyBytes, &tj); err != nil {
        // 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))
    }

    errorCode := twirp.ErrorCode(tj.Code)
    if !twirp.IsValidErrorCode(errorCode) {
        msg := "invalid type returned from server error response: " + tj.Code
        return twirp.InternalError(msg)
    }

For example:

if tj.Code == "" {
        return twirpErrorFromIntermediary(statusCode, statusText, string(respBodyBytes))
    }

after that block would provide the behaviour that I would expect of mapping the http response to the correct twirp error.

[timclark-bgch]

I think https://github.com/twitchtv/twirp/blob/337e90237d72193bf7f9fa387b5b9946436b7733/protoc-gen-twirp/generator.go#L528

is the section of code that needs modification.

[spenczar]

Wow, this must have been a nasty bug to figure out. Thanks for the issue report - I agree with your proposed fix.

[timclark-bgch]

Do you want me to create a pull request for you? I can have one ready sometime today.

[spenczar]

That would be great, yes.

[LindseyB]

I'm running into this also