Marak / translate.js

translate text from one language to another on node.js and the browser. 30+ languages supported, simple as cake.
310 stars 60 forks source link

single quote cases exception #9

Closed dylang closed 9 years ago

dylang commented 13 years ago

When translating a string in Node with a single quote I get the following exception:

/usr/local/lib/node/.npm/translate/0.6.0/package/lib/translate.js:80
  callback(null, JSON.parse(body).responseData.translatedText);
                                              ^
TypeError: Cannot read property 'translatedText' of null
at [object Object].callback (/usr/local/lib/node/.npm/translate/0.6.0/package/lib/translate.js:80:51)
at IncomingMessage.<anonymous> (/usr/local/lib/node/.npm/request/1.9.3/package/main.js:253:21)
at IncomingMessage.emit (events.js:59:20)
at HTTPParser.onMessageComplete (http.js:111:23)
at Socket.onend (http.js:1224:12)
at Socket._onReadable (net.js:630:26)
at IOWatcher.onReadable [as callback] (net.js:156:10)
Marak commented 13 years ago

Hrmmm, let's see what we can do about that.

dylang commented 13 years ago

This was my local workaround, Google doesn't seem to care, at least with English.

 str.replace(/'/g, '’')
Marak commented 13 years ago

Can you paste me the sample code which caused the issue? I'm trying to reproduce locally and cannot.

dylang commented 13 years ago

This test works on the command line. If you see it working when there are no single quotes, then it breaks with the single quote.

$ node
> var t = require('translate');
> t.text("this is a test", function(err, text) { console.log(text); });
> esta es una prueba
> t.text("this isn't a test", function(err, text) { console.log(text); });
> 
/usr/local/lib/node/.npm/translate/0.6.0/package/lib/translate.js:80
      callback(null, JSON.parse(body).responseData.translatedText);
                                                  ^
TypeError: Cannot read property 'translatedText' of null
    at [object Object].callback (/usr/local/lib/node/.npm/translate/0.6.0/package/lib/translate.js:80:51)
    at IncomingMessage.<anonymous> (/usr/local/lib/node/.npm/request/1.9.3/package/main.js:253:21)
    at IncomingMessage.emit (events.js:59:20)
    at HTTPParser.onMessageComplete (http.js:111:23)
    at Socket.onend (http.js:1224:12)
    at Socket._onReadable (net.js:630:26)
    at IOWatcher.onReadable [as callback] (net.js:156:10)
Marak commented 13 years ago

Interesting....the same code works fine for me on the repl and in a file.

Can you try adding a console.log line here: https://github.com/Marak/translate.js/blob/master/lib/translate.js#L76 to dump the body?

I'd like to see what google is returning that causing the JSON.parse to bomb.

Also, which version on node are you running?

dylang commented 13 years ago

Node 0.4.2.

I'm using translate from npm - not locally.

I'll download the source from trunk and see what happens.

Marak commented 13 years ago

npm still installs the source on your machine @ /usr/local/lib/node/.npm/translate/0.6.0/package/lib/translate.js

but cloning the source and running

 npm link . 

from inside the repo will link your clone into your local npm

dylang commented 13 years ago

Yup but was bringing up in case the version on npm was out of date because you said it worked for you locally. I tried the version from trunk and had the same problem.

> var t = require('./translate.js');  t.text("this isn't a test", function(err, text) { console.log(text); });
> 
/Users/dylan/projects/translate.js/lib/translate.js:80
      callback(null, JSON.parse(body).responseData.translatedText);
                                                  ^
TypeError: Cannot read property 'translatedText' of null
    at [object Object].callback (/Users/dylan/projects/translate.js/lib/translate.js:80:51)
    at IncomingMessage.<anonymous> (/usr/local/lib/node/.npm/request/1.9.3/package/main.js:253:21)
    at IncomingMessage.emit (events.js:59:20)
    at HTTPParser.onMessageComplete (http.js:111:23)
    at Socket.onend (http.js:1224:12)
    at Socket._onReadable (net.js:630:26)
    at IOWatcher.onReadable [as callback] (net.js:156:10)

Request:

{ uri: 'http://ajax.googleapis.com/ajax/services/language/translate?v=1.0&q=this%20isn\'t%20a%20test&langpair=en|es' }

Response:

{"responseData": null, "responseDetails": "invalid language pair", "responseStatus": 400}

It looks like node is doing incorrect html escaping of '?

Marak commented 13 years ago

Can you try updating to the latest version of request by running:

 npm install request

Thanks

dylang commented 13 years ago

Nevermind about the escaped single quote - that's the console trying to be helpful.

Real url:

http://ajax.googleapis.com/ajax/services/language/translate?v=1.0&q=this%20isn't%20a%20test&langpair=en|es

I have the latest request:

request@1.9.3             =mikeal active installed latest remote   Simplified HTTP request client.  

I'm a little insane about updating my npm modules, i run npm update every morning, afternoon, and evening.

dylang commented 13 years ago

I don't understand the purpose of this:

// http client for accessing google api
var googleTranslate = http.createClient(80, 'ajax.googleapis.com');

If you are using request you don't need to create another connection with node's http.

Marak commented 13 years ago

Yeah, must be old code from before request module existed, I removed it.

Are you still experiencing an issue? I can't figure out how to replicate it on this machine. The url that is being sent to request is correct ( i.e. it works if you curl it or paste it in a browser ).

dylang commented 13 years ago

Still having the issue from Node, and I agree - that same url works in the browser. So strange. I'm going to try sending different user agent to see if maybe Google is doing something.

dylang commented 13 years ago

Hmm - getting somewhere - that url gives me the same error when I run it through curl.

dylang commented 13 years ago

Fixed it!

You have some strangeness in your url builder. encodeURIComponent only has one input, a string. Also you were doing string building and array concat which is kind of confusing and not a helpful optimization with just a few strings. Finally I swapped the order so that if there is anything in the string that confuses Request or Google at least Google still gets the language and translates it.

Here's what I have:

return '/ajax/services/language/translate?v=1.0'
        + '&langpair=' + encodeURIComponent(input + '|' + output)
        + '&q=' + encodeURIComponent(text);

Do you want me to create a pull request?

Marak commented 13 years ago

Hrmmm, sure. I wonder why it works with on one machine, but not another.

dylang commented 9 years ago

haha node 0.4.2. this is old.