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

Return error from twirp.WriteError #201

Closed
ofpiyush opened this issue Dec 3, 2019 · 2 comments
Closed

Return error from twirp.WriteError #201

ofpiyush opened this issue Dec 3, 2019 · 2 comments

Comments

@ofpiyush
Copy link
Contributor

ofpiyush commented Dec 3, 2019

Moving discussion from the PR as @spenczar requested.

Recap:

Current implementation of twirp.WriteError does not return any error occurred while writing the TwirpError.

Humor

WriteError does not return the error if an error occurs while writing the error.

Since the public API currently returns nothing, all existing code looks like

twirp.WriteError(resp, err)

As no return is being processed, adding a return to this method will not break existing code.

@spenczar
Copy link
Contributor

spenczar commented Dec 3, 2019

You're right that it won't break most code. The only exception would be that the function's type changes, so if someone has a var w func(*http.ResponseWriter, error) = twirp.WriteError, that code will break. That seems pretty unusual, and the function is new enough that it might be safe to modify without any significant changes.

It's a bit of a violation of semver to make this part of a minor release, but I think it might be a forgivable one.

@spenczar
Copy link
Contributor

spenczar commented Dec 3, 2019

Oh, and in case it isn't obvious - yes, it should return an error if it fails to write.

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

No branches or pull requests

2 participants