Re: [Q] "Should be empty:" when using reject()

[Kris Kowal]

A question first. Are you using Q as a <script> or in Node, or using
some other loader? There is some feature detection behind this message
that might be awry.

The message should only every be written to the console in a browser,
the first time a rejected promise is constructed. It is a mechanism to
prevent unhandled rejections from going unnoticed, which can happen if
the programmer forgets to terminate a chain of promises with .done(),
.end(), or .nodeify(). Unfortunately, once the message has been
written to the console, it cannot be removed. However, the browser
console provides a living view into the content of the array. When
the rejection gets handled, Q removes the "reason" from the array.
Thus, if you see Should be empty: [] on your console, nothing is
wrong.

As always, we have our eyes open for a better solution, but it will
require support from the console / web inspector.

Kris Kowal

[alphanull]

First of all: thank you very much for the quick reply!

Am Mittwoch, 31. Oktober 2012 19:26:16 UTC+1 schrieb Kris Kowal:

A question first. Are you using Q as a <script> or in Node, or using
some other loader? There is some feature detection behind this message
that might be awry.

I use Q as a requireJS module in the Browser.
 

It is a mechanism to prevent unhandled rejections from going unnoticed, which can happen if
the programmer forgets to terminate a chain of promises with .done(),
.end(), or .nodeify().

First of all, the rejection should be handled, at least the corresponding function is called and this is how the code for this looks like (shortened):

myDefferedFunc.then(    
                function (data) { ... },
                function (status) { ... }
 );

I did not "terminate" anything, but I was under the impression that this is not a mandatory step, or is it?
 

Unfortunately, once the message has been
written to the console, it cannot be removed. However, the browser
console provides a living view into the content of the array.  When
the rejection gets handled, Q removes the "reason" from the array.
Thus, if you see Should be empty: [] on your console, nothing is
wrong.

Actually, after the message "Should be empty" the array object logged is not empty, but simply populated with the object I used as param for rejected:

deferred.reject({status: 123});  --> console message: "Should be empty [Object]" --> [Object] is just "{ status: 123 }"

but if I only state: deferred.rejected(); (without any params) then it reads: "Should be empty [Error]"   --> the "Error" Object seems to be just a plain Error Object with no message.

Still have no clue why this happens, maybe I am just messing something up, but after all I am completely new to "Q", so please bear with me ;)

[alphanull]

PS: it also does not seem to matter if I return "deferred.reject()" immediatly or if I do somtheing like this:

 var deferred = Q.defer();          
 setTimeout(function () { deferred.reject() } ,1000);          
 return deferred.promise;

Of course in this case the error callback is executed later, and so does the "Should be emtpy" message, but overall the result is the same.

[Domenic Denicola]

As part of the release that includes `done`, we should probably document the capping procedure and the problem it solves in depth (like the WinJS documentation page).

Also, "Should be empty" is not a great prefix. Maybe "[Q] Unhandled rejection reasons (see http://bit.ly/q-rejection):"

Finally, as for console hooks: browser extensions are not too hard to write these days. Worth looking in to, I suppose.

[alphanull]

OK, as far as I understand this message is intended to catch any unhandled rejections so that they can be noticed by the developer. But then this message should not appear, because the rejection _is_ actually handled, at least the corresponding function is called as expected. In this case can I regard this as a "false positive" and safely ignore it, or does that point to some kind of bug? And if yes, is it my fault or something with Q?

[Kris Kowal]

I’m pretty sure there is an unhandled error. The .end() is mandatory
unless you’re returning the promise from a function. You will know
your code is working when [] is empty.

We might be able to help more if you post a gist with the smallest
amount of code that causes the failure case. We might be able to point
out a problem in the usage. For example, it’s pretty uncommon to use
the result of deferred.reject. You might mean Q.reject, but we won’t
know for sure without a larger sample.

Kris Kowal

[Domenic Denicola]

Which browser? That might actually have an impact.

[alphanull]

Which browser? That might actually have an impact.

Yes, it does! Just tested in Various Browsers and these are the results:

Firefox (all): "Should be empty: []"
Chrome 22: "Should be empty: []"
Chrome 8: "Should be empty: [Object, Object]" (!!!)
Opera  11,12: "Should be empty: [Object]"
Safari 4,5 : "Should be empty: [Object]"
IE8, 9: "Should be empty"

So Opera and Safari (think we can ignore old Chrome) both get it wrong. Well, now it smells more like a browser issue, does it? Still interesting that the message appears at all in any case. Is that intended?

[Kris Kowal]

On Wed, Oct 31, 2012 at 1:24 PM, alphanull <kude...@alphanull.de> wrote:
> So Opera and Safari (think we can ignore old Chrome) both get it wrong.
> Well, now it smells more like a browser issue, does it? Still interesting
> that the message appears at all in any case. Is that intended?

It is, but only if it’s going to be accurate. Unfortunately, it looks
like that means user agent sniffing, which I don’t intend to do.

Darn.

Domenic, I agree that it needs a better label, and that we should
invest some time in making browser extensions that make the promise
graph visible.

Kris

[alphanull]

I’m pretty sure there is an unhandled error. The .end() is mandatory
unless you’re returning the promise from a function.

Still happens when I use a promise and actually defer the rejection using a timeout.
 

For example, it’s pretty uncommon to use
the result of deferred.reject.

Yes, this might be due to my inexperience. The reason I do this is because I am implementing a "storage" module which can save a javascript object using various "adapters", including async operations (xhr) and sync operations (localStorage). So if I do something like:

this.adapter.read(id).then(            
                function (status) { ... success ... },
                function (status) { ... error .... }
);

It is not obvious (and shouldn't be!) if the resulting operation is sync or async. Since i find it a bit weird to use a setTimeout to return the result from the localStorage adapter, I have the need of immediatly returning the result of the operation. This is actually done - simplified - like this:

var deferred = Q.defer(),
data = localStorage.getItem(key);
return (data !== undefined) ? deferred.resolve(data) : deferred.reject({ status: 404 });

In the case of the xhr adapter of course just a promise is returned, which can be resolved later like ususal. But I like my storage module to be agnostic about the adapter and if it is synchronous or asynchronous.

So, what would be a more appropriate way to do this?

[Kris Kowal]

On Wed, Oct 31, 2012 at 1:37 PM, alphanull <kude...@alphanull.de> wrote:
> var deferred = Q.defer(),
> data = localStorage.getItem(key);
> return (data !== undefined) ? deferred.resolve(data) : deferred.reject({
> status: 404 });

This would suffice since you aren’t resolving in a future turn:

var data = localStorage.getItem(key);
return data !== undefined ? Q.resolve(data) : Q.reject({status: 404});

If you were resolving in a future turn you would actually need
Q.defer() and it would look like:

var deferred = Q.defer();
localStorage.getItem(key, function (data) {
if (data !== undefined) {
deferred.resolve(data);
} else {
deferred.reject({status: 404});
}
});
return deferred.promise;

Bear in mind that returning a promise strongly corrales the user into
using the data asynchronously, regardless of whether the promise is
resolved in the same turn or a future turn. You will never need to
explicitly resolve in a setTimeout block. Q wraps your callbacks
implicitly, and usually in a way that performs better than setTimeout.

Kris Kowal

[alphanull]

Kris,

thanks a lot for making this clear! Your solution works and in this case is of course more appopriate, but the different browser behavior unfortunately remains.

Bear in mind that returning a promise strongly corrales the user into
using the data asynchronously, regardless of whether the promise is
resolved in the same turn or a future turn.

Yes I know, but I think in this special case it is justified, because the module using the storage cannot and should not know about the synchronizity of the resulting concrete storage operation. Since my storage module will also be able to load and save data to several sources concurrently, it is also sometimes not known if the data comes from a remote source or if it was cached in localStorage before. But the module can just use the same API, which is quite elegant imho. Also it is absolutely OK if every operation appears to be asynchronous even if it is not, since asynchronity is and has to be always assumed in this case.

[Gili T.]

Kris,

I think alphanull is right. The unhandled error detection is broken. I haven't figured out a minimum testcase yet but I can definitely confirm getting the "unhandled rejected reasons" warning (in my case, containing a single entry) followed by my rejection handler getting fired.

Is it possible something like this happens?

1. A function returns "new Promise().then(A).then(B).thenC()"

2. Q reaches the end of the chain and notices the exception is not caught, so it prints the warning.

3. But, the Promise is returned by the function and a higher level function registers an exception handler using then(), catch() or done().

4. Q fires the exception handler.

Meaning, I think that the warning is getting fired before Q actually reaches the end of the chain. Any ideas?

Gili