1511692 - Extremely spammy warning |WARNING: 'NS_FAILED(rv)', file [snip]/xpcom/io/NonBlockingAsyncInputStream.cpp, line 171|

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P5)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1435899 +++

We see this warning thousands of times in our test suite:
https://taskcluster-artifacts.net/byD651nKS3unzZJq4IzNHQ/0/public/logs/live_backing.log

It comes from here:
https://hg.mozilla.org/mozilla-central/rev/5461b62599d7#l1.14

Comment 1

[ISHIKAWA, Chiaki]

Thank you for filing this.  I am not sure if the FF application generates this warning in the DEBUG BUILD as  TB's DEBUG BUILD of TB.

But I think we can discards "NS_WARN_IF()" call in the if-expression once the situtation is assessed.

Comment 2

[Honza Bambas (:mayhemer)]

It's perfectly legal to return an error (e.g. NS_BASE_STREAM_CLOSED) for Available().  It's usually not fatal, only indication of a legal state.

Comment 3

[ISHIKAWA, Chiaki]

Attachment: remove NS_WARN_IF() call to suppress spammy warning.

If it is legit to return an error, then we can safely remove the call to NS_WARN_IF().

Suggested patch attached.
It compiles locally under linux.

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 9029366 [details] [diff] [review]
remove NS_WARN_IF() call to suppress spammy warning.

Review of attachment 9029366 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/io/NonBlockingAsyncInputStream.cpp
@@ +167,5 @@
>  
>  NS_IMETHODIMP
>  NonBlockingAsyncInputStream::Available(uint64_t* aLength) {
>    nsresult rv = mInputStream->Available(aLength);
> +  if (NS_FAILED(rv)) {

I don't think this will be approved since the idea is to hear about errors. I think first we need to find out what the specific error is and then ignore it.

Comment 5

[Jorg K (CEST = GMT+2)]

I added some debug and the error returned is 0x80470002 which is NS_BASE_STREAM_CLOSED. Oh, that was already predicted. So I would code something like
nsresult rv = mInputStream->Available(aLength);
if (rv == NS_BASE_STREAM_CLOSED || NS_WARN_IF(NS_FAILED(rv)) {
  return rv;
}

Comment 6

[Jorg K (CEST = GMT+2)]

Attachment: 1511692-suppress-warning.patch - When landing, change to r=baku

Comment 7

[ISHIKAWA, Chiaki]

(In reply to Jorg K (GMT+1) from comment #5)
> I added some debug and the error returned is 0x80470002 which is
> NS_BASE_STREAM_CLOSED. Oh, that was already predicted. So I would code
> something like
> nsresult rv = mInputStream->Available(aLength);
> if (rv == NS_BASE_STREAM_CLOSED || NS_WARN_IF(NS_FAILED(rv)) {
>   return rv;
> }

I see. Thank you.

Comment 8

[Jorg K (CEST = GMT+2)]

Thanks, Andrea!

Dear sheriff, please change reviewer to r=baku when landing.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e77b8931ae7a
suppress warning for NS_BASE_STREAM_CLOSED from base streams Available() call. r=baku

Comment 10

[Honza Bambas (:mayhemer)]

Comment on attachment 9029389 [details] [diff] [review]
1511692-suppress-warning.patch - When landing, change to r=baku

Review of attachment 9029389 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, that's exactly what I wanted to suggest!  thanks.

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/e77b8931ae7a