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

Initial implementation of go-translate #1

Merged
merged 14 commits into from Mar 22, 2019

Fix missing returns in error cases

  • Loading branch information
yrliou committed Mar 20, 2019
commit 81590b7d33d4343879bf957fd5c2e7e0c4d62db0
@@ -67,6 +67,7 @@ func GetLanguageList(w http.ResponseWriter, r *http.Request) {
req, err := http.NewRequest("GET", MSTranslateServer+languageEndpoint, nil)
This conversation was marked as resolved by yrliou

This comment has been minimized.

Copy link
@jumde

jumde Mar 21, 2019

Collaborator

We unset the X-Forwarded-* headers sent to Google servers at the proxy. We should unset the headers for connection to Microsoft servers as well. See: https://github.com/brave/devops/pull/785/files#diff-e262328707cf31665d8378e0949ef286R67

This comment has been minimized.

Copy link
@evq

evq Mar 21, 2019

Member

they aren't automatically set. since we're not explicitly setting them on the Request struct I think no change is needed?

This comment has been minimized.

Copy link
@yrliou

yrliou Mar 21, 2019

Author Member

@jumde This is a newly created request and won't have headers from google request, and I dump the request here to double check, we only have Host: api.cognitive.microsofttranslator.com header here.
Does this answer your concern here?

if err != nil {
http.Error(w, fmt.Sprintf("Error creating MS request: %v", err), http.StatusInternalServerError)
This conversation was marked as resolved by yrliou

This comment has been minimized.

Copy link
@jumde

jumde Mar 20, 2019

Collaborator

return here if there is an error

This comment has been minimized.

Copy link
@yrliou

yrliou Mar 20, 2019

Author Member

addressed in 81590b7

return
}

client := getHTTPClient()
@@ -99,6 +100,7 @@ func GetLanguageList(w http.ResponseWriter, r *http.Request) {
msBody, err := ioutil.ReadAll(msResp.Body)
if err != nil {
http.Error(w, fmt.Sprintf("Error reading MS response body: %v", err), http.StatusInternalServerError)
return
}
body, err := language.ToGoogleLanguageList(msBody)
if err != nil {
@@ -154,6 +156,7 @@ func Translate(w http.ResponseWriter, r *http.Request) {
msBody, err := ioutil.ReadAll(msResp.Body)
if err != nil {
http.Error(w, fmt.Sprintf("Error reading MS response body: %v", err), http.StatusInternalServerError)
return
}
body, err := translate.ToGoogleResponseBody(msBody, isAuto)
if err != nil {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.