#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 )
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)
Change History (8)
Changed 12 years ago by
Attachment: | Deferred_conditionalErrorLogging_rev18640#9500.patch added |
---|
Changed 12 years ago by
Attachment: | xhr_conditionalErrorLogging_rev18640#9500.patch added |
---|
patch on dojo/_base/xhr.js
comment:1 Changed 12 years ago by
Cc: | Eugene Lazutkin added |
---|---|
Description: | modified (diff) |
Owner: | changed from anonymous to James Burke |
comment:2 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
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 11 years ago by
Milestone: | tbd → 1.4 |
---|
patch on dojo/_base/Deferred.js