Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9500 closed defect (fixed)

allow xhr deferred not to always loggin error

Reported by: remy damour Owned by: James Burke
Priority: high Milestone: 1.4
Component: Core Version: 1.3.1
Keywords: Cc: Eugene Lazutkin
Blocked By: Blocking:

Description (last modified by Eugene Lazutkin)

Hi,

I'm using xhr-deferreds and each time I call .cancel(), and error gets logged. I tried providing my errorHandler but without success.

I found the explanation in dojo.js (full uncompressed file), dojo.xhr() automatically register _deferError as first errback, which single purpose is to log the error within console:

var _deferError = function(/*Error*/error, /*Deferred*/dfd){
  //summary: errHandler function for dojo._ioSetArgs call.
  console.error(error);
  return error;
}

As being trapped in a closure, we currently cannot prevent its registration, and since it's the first errback to be registered, it is the first to be triggered upon error.

I understand why this is useful (it's pretty handy to have first errback log your error within console automatically) but I don't understand why we cannot make error logging optional.

In my case, when I call cancel, I don't want any error to be logged, since in these cases, I don't consider canceling my deferred being an error.

I've enclosed diff files against last revision (18640) that would allow to make error logging optional.

Regards, Remy

Attachments (2)

Deferred_conditionalErrorLogging_rev18640#9500.patch (300 bytes) - added by remy damour 10 years ago.
patch on dojo/_base/Deferred.js
xhr_conditionalErrorLogging_rev18640#9500.patch (362 bytes) - added by remy damour 10 years ago.
patch on dojo/_base/xhr.js

Download all attachments as: .zip

Change History (8)

Changed 10 years ago by remy damour

patch on dojo/_base/Deferred.js

Changed 10 years ago by remy damour

patch on dojo/_base/xhr.js

comment:1 Changed 10 years ago by Eugene Lazutkin

Cc: Eugene Lazutkin added
Description: modified (diff)
Owner: changed from anonymous to James Burke

comment:2 Changed 10 years ago by Eugene Lazutkin

Description: modified (diff)

comment:3 Changed 10 years ago by Eugene Lazutkin

It looks like this error logging should depend on isDebug value. I can see the value of it during debugging, but it clearly should be off in production.

comment:4 Changed 10 years ago by Eugene Lazutkin

And this is the code in question: http://bugs.dojotoolkit.org/browser/dojo/trunk/_base/xhr.js#L584

In general xhr.js uses console in twp places without distinction between debugging and production. Was it the intent to warn innocent end users on different aspects of XHR? Like in http://bugs.dojotoolkit.org/browser/dojo/trunk/_base/xhr.js#L242:

if(!dojo.config.useCommentedJson){
  console.warn("Consider using the standard mimetype:application/json."
    + " json-commenting can introduce security issues. To"
    + " decrease the chances of hijacking, use the standard the 'json' handler and"
    + " prefix your json with: {}&&\n"
    + "Use djConfig.useCommentedJson=true to turn off this message.");
}

comment:5 Changed 10 years ago by James Burke

Resolution: fixed
Status: newclosed

Fixed by toonetown as part of #9618, you can now pass a failOk param to the xhr calls to avoid the error logging. As for the other console.warn, we'll leave it in, I do not want to add extra bytes for an isDebug check, and it is only visible if the person is a developer anyway with an open console. useCommentedJson should be removed in a 2.0 Dojo anyway.

comment:6 Changed 10 years ago by bill

Milestone: tbd1.4
Note: See TracTickets for help on using tickets.