-
Notifications
You must be signed in to change notification settings - Fork 132
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
JSON type not detected properly #34
Comments
This bug has hit me too, but different variation. The API service I call responds with HTTP Content-type response: application/json;charset=UTF-8 Inside your code, node-rest-client.js, you check for isJSON, by doing a character comparision that only supports lowercase against two values: application/json;charset=utf-8 Hence this is causing my data to be returned as a String, and not as a parsed JSON object as expected. Please can you update the function:
To support "content" being parsed to lower case before doing the check comparison? Thanks very much. |
Please take a look at this - http://stackoverflow.com/questions/7718476/are-http-headers-content-type-c-case-sensitive?rq=1 It is slightly unclear, but I believe the part that states - http://www.w3.org/TR/html4/charset.html#h-5.2.1 says "Names for character encodings are case-insensitive". Should mean UTF and utf would both be supported. |
Hi you can customize the content/type returned by server passing it as a parameter in client initialization (parameter "mimetypes") please see https://github.com/aacerox/node-rest-client#options-parameters. So you can set responses to be parsed as json, customizing json content-type header returned by server in the following way: var options ={
// customize different content-types for json or xml connections, separated by comma
mimetypes:{
json:["application/json","application/json;odata=verbose;charset=utf-8"]
}
};
var client = new Client(options) Hope this help. |
Already had that in place, thanks for the suggestion:
|
Doesn't work. Neither does:
|
The "jsonctype" variable inside isJSON continues to only display two values: application/json;charset=utf-8 and hence fails because UTF is in uppercase in the content-type response from server. Please can you explain how I can augment "jsonctype" to either be case insenstive, or load "application/json;charset=UTF-8" as another argument in its array of values. |
Will you re-open this issue ? I don't think it should be closed. Otherwise your client does not support Grails Applications correctly. In fact, there seems to be more weight behind capitalization than lowercase. |
For anyone else's benefit, here is hackish work-around:
|
I prefer @aacerox |
@arcseldon I've tried by myself and mimetype options customization works properly so I think there are no reasons to reopen the issue, but if you want I'll try to help you debug the client. Could you please enable debug mode in client by setting env variable DEBUG to true (export DEBUG=true or set DEBUG=true) then call the client passing it mymetype initialization paremeter (as you said you had done) and send me the result of your client invocation?? |
Thanks for offering to take a look. Here is the DEBUG output as requested. You will see that initially jsonctype is: jsonctype: (I need the uppercase UTF-8 declaration). But further down in the trace, it is: jsonctype: [ 'application/json', 'application/json;charset=utf-8' ], And it is always just [ 'application/json', 'application/json;charset=utf-8’ ] inside isJSON function check when looking at the variable assignments in debugger. (i have removed some time stamps and auth username/password info only) Cheers, [API CLIENT] -> ConnectManager { xmlctype: [ 'application/xml', 'application/xml;charset=utf-8' ], [API CLIENT] -> ConnectManager { xmlctype: [ 'application/xml', 'application/xml;charset=utf-8' ], [API CLIENT] -> options pre connect { host: 'localhost', [API CLIENT] -> args = { path: { id: 1 }, [API CLIENT] -> options pre connect { host: 'localhost', [API CLIENT] -> options data undefined [API CLIENT] -> content-type: application/json;charset=UTF-8 [API CLIENT] -> content-encoding: undefined [API CLIENT] -> not compressed On 28 Apr 2014, at 23:07, Alejandro Alvarez Acero notifications@github.com wrote:
|
Find that debug at the end:
odd. Here is the Client setup code:
Presumably, that would be all that is required to ensure the options were setup? On 28 Apr 2014, at 23:27, Richard Seldon arcseldon@gmail.com wrote:
|
Hi If you don't mind we can do the following trying to find a solution:
"isJSON":function(content){
debug("JSON content", content);
debug("JSON jsonctype", this.jsonctype);
var result = false;
for (var i=0; i<this.jsonctype.length;i++){
result = this.jsonctype[i] === content;
if (result) break;
}
return result;
},
options = {
mimetypes: {
json: ["application/json", "application/json;charset=utf-8", "application/json;charset=UTF-8"]
},
user: config.api.username,
password: config.api.password
}; // in your code this was a comma not a semicolon
// use var keyword to declare variable.
var client = new Client(options); The "undefined" you have found in the traces doesn't have to be with our problem (it's realated to JSON data posted when use POST, PUT or PATCH) |
Hi Alejandro, Thanks again for your support! Will do as you suggest, here is a bit of code to make it clear what I am doing by way of calling. The comma you picked up is nothing I believe, I am just declaring a single ‘var’ at the top, then comma-separating out the rest - but will try your approach below anyhow: set up code:var mailSender = require('../../common/utilities/mail')(config), calling function itself:var getAgencyById = function (id) { Mocha test with Chai:describe('getAgencyById', function () { I have about another 40-50 functions that all use the node-rest-client, all with accompanying functional tests that all pass - just this issue around not getting a JSON Object, versus a string representation. Not a show stopper but certainly be nice to solve it prior to going into PRD - this is a commercial, cloud solution. Cheers, On 29 Apr 2014, at 00:17, Alejandro Alvarez Acero notifications@github.com wrote:
|
Ok, the results of those two line DEBUG Statements are as follows: [API CLIENT] -> JSON content application/json;charset=UTF-8 Switching the var stuff around: [API CLIENT] -> JSON content application/json;charset=UTF-8 Ie. no difference. Any thoughts? On 29 Apr 2014, at 00:17, Alejandro Alvarez Acero notifications@github.com wrote:
|
Hi If I understood right, in your code first you initialize the client as you describe in "set up code", then you use that client several times, and then comes the "problematic" function as you describe in "calling function itself", right?. Maybe there's another client instantiation between first initialization and problematic function call, that reset "content-types" to its default values? |
Hi, Couple of important European Champions League games for Spain over next couple of days! Hope it will be Real Madrid vs Chelsea in the final, but we shall see. Every team in the semi-finals still has a chance, and Chelsea are missing half their squad for the second round tie against Athletico. Re. location - Am in Japan hence the strange timestamps you might be seeing - 2 am here atm. Re code … lol Am running two Specs in the same folder (ie. two files each with their own set of API tests). Yes, sharing a Client object for each test, but not (deliberately) changing any options etc on that Client anywhere. Am going to comment out all the tests in one file, and all tests except one in the other and see what happens now. Give you the results shortly. Again, thanks for the help. On 29 Apr 2014, at 01:53, Alejandro Alvarez Acero notifications@github.com wrote:
|
Ok, solved. 2:17:26 [API CLIENT] -> JSON content application/json;charset=UTF-8 So, now it is present, and the test works: describe('getAgencyById', function () { I think Mocha must be concurrently running both test Spec files in a way that is causing the Client options object to be overwritten somehow. Let me ask you this - if I create a Client object inside a Module ie. module.exports = function (config) { // create Client object here // declare method(s) that internally reference the Client object (and can access it via closure) return { } Is that Client object in global namespace??? I thought it would be encapsulated in something similar to a function scope, and therefore be unaffected by another module declaring a Client object of the same name …? The only “other” explanation is somehow that your code is referencing the Options object in Global namespace, and subsequent invocations passing in a different Options object overwrite that one. Please discount this possibility if you know for a fact that definitely is not happening. Cheers, On 29 Apr 2014, at 01:53, Alejandro Alvarez Acero notifications@github.com wrote:
|
Hi Alejandro, Quick question, as this would affect the design of my API layer, and is directly related to this. Is it possible to declare two different Client objects, each with their own Options and use these at the same time (both located inside their own application modules)? I just want confirmation that by design attributes such as Options do not get overwritten by each other - as the results of this bug appear to indicate. You are not altering prototype info etc are you that would basically update all object instances? Secondly, please let me know, it is safe to reuse the Client object right? I am not defining a new one with each function API call, rather I am instantiating one per module (API file), and then reusing that between all methods in the same module - they all just make calls on it passing in their own args (without changing the Options object). Would really appreciate clarfication on the above. Thanks very much. |
Hi My intention when I create node-rest-client was that you could instantiate different autonomous instances of the client that obviously do not share configuration parameters, and as you said, with the target that you could use one instance of the client through many invocations, with the restriction that once the client has been initialized you can't change it's initialization parameters and that's exactly how client works. As you can see in the following example you can use the client in different modules or in different instances and none of them share configuration parameters. In the case of the module pattern, each module maintains its own local scope where client is declared, so instances of that client created in different modules should never share any information. Instantiating the client in the global scope acts the same way, each variable has its own reference to different instances of the client that do not share any information at all. var Client = require('../lib/node-rest-client.js').Client;
// declare as different instances
var client1 = new Client({
mimetypes:{
json:["application/json;charset=UTF-8;client1"],
xml:["application/xml;charset=UTF-8;client1"]
}
}),
client2 = new Client({
mimetypes:{
json:["application/json;charset=utf-8;client2"],
xml:["application/xml;charset=utf-8;client2"]
}
});
// declare clent inside different module scopes
module.exports.moduleA=function(config){
var client = new Client({
mimetypes:{
json:["application/json;charset=UTF-8"],
xml:["application/xml;charset=UTF-8"]
}
});
return {
"options":function(){return client.options}
};
};
module.exports.moduleB=function(config){
var client = new Client({
mimetypes:{
json:["application/json;charset=utf-8"],
xml:["application/xml;charset=utf-8"]
}
});
return {
"options":function(){return client.options}
};
};
var modA = new module.exports.moduleA(),
modB = new module.exports.moduleB();
console.log("================= module example =======================");
console.log("moduleA options",modA.options());
console.log("moduleB options",modB.options());
console.log("================= instance example =======================");
console.log("client1 options", client1.options);
console.log("client2 options", client2.options); Maybe there's one place in your code where one previously declared global client var is reassigned inadvertently with another one overwriting first one. To check this you can create hashcodes for each client instance and check if the client is overwritten somewhere: client1.hashcode = new Date().getTime() + 17;
client2.hashcode = new Date().getTime() + 31; Hope you find a solution soon. Best regards from Spain. |
Hi, Lol - you had to be an Atletico supporter. Well the good news is that you are football supporter! Best of luck, Atletico are having a great season, a lot is being said of the Manager. Hope you win the league, but Chelsea win the ECL. Mind you, think Atletico are favourites to progress to the final - Chelsea got no away goal advantage and most of the best players cannot play either through injury or card penalties. But with Mourinho you never know.. Thanks for the detailed reply - that answered precisely what I was asking about and left no doubts about intended behaviour. Please add that to your official documentation on home page - it is really useful info! Ok, so I put in hash codes, and yes, definitely each client instantiated (one per Spec) is keeping its own Client (the hash codes always match up). Also, inspecting the debug properties at runtime, I can see that the Options for each is also correctly assigned. However, alas, the isJSON values is still wrong - it overwrites the jsonctype with whatever Spec gets executed (Client created) last. In other words, you definitely have a bug in your application. this.jsonctype is NOT reading the Options.mimetypes.json values from the currently executing Client. It is somehow referencing whatever the Options.mimetypes.json value was for the last executed instantiation of Client. Don’t have time to debug your code right now, as what I need is working on the basis that ALL my Clients have the same mime type values (they do now) - and I have to get a release done by tomorrow. But if I get some downtime, be happy to debug and determine where your code contains this bug. It is NOT in my client code, if the information you provided below is the expected behaviour. Good evening from Tokyo, Japan :) On 29 Apr 2014, at 07:05, Alejandro Alvarez Acero notifications@github.com wrote:
|
I understand this issue has been closed, but I would like to ask one small question. First, I think the options literal is a great way to customize mapping options for JSON mime types. How ever, I think that we could reduce how often we need to use custom options by making the content-type comparison case insensitive. What is wrong with the following amendment to the isJSON() method:
I'm writing a cookbook for Packt and I wanted to include the node-rest-client (best I've seen while doing my research). I want to make a call to an open source web service that returns JSON, however it's coming back with a content-type of "application/json;charset=UTF-8". I can include the mimetypes option for Client, but it seems like a case-insensitive match might prevent a large number of unnecessary overrides in the first place. I was just wondering why a case-insensitive comparison wasn't used. Thanks for the awesome work! |
Best of luck with the cookbook. Will look out for that when it gets published Just wish to add that node-rest-client has been an excellent choice, and would fully support your decision to include it based on practical usage. The ‘issue’ around isJSON options has been the only small discrepancy - everything else has been out of the box. On 25 May 2014, at 06:37, @WebCoding4Fun notifications@github.com wrote:
|
Hi OakRaven First of all thanks for your support. You're right, I didn't notice at all that the comparision was done in a case sensitive manner. Tonight I'll modify the code to make a case INsensitive comparision in both "isJSON" method and "isXML" method. Thanks for your help. |
what about different charset? I think you should parse the header and keep the charset or maybe convert to utf8, like this module https://www.npmjs.com/package/content-type |
On of the services my app relies upon suddenly changed the mime type to
application/json;odata=verbose;charset=utf-8
(probably some redmondish attempt to screw up something not invented by them).As a result the body of the message is not interpreted as JSON anymore. So we should either match this obscure mimetype, or match on a substring.
The text was updated successfully, but these errors were encountered: