"ASSERTION: cancel with non-failure status code" with XMLHttpRequest, moz-blob




3 years ago
2 years ago


(Reporter: jruderman, Unassigned)


(Blocks: 1 bug, {assertion, testcase})

assertion, testcase
Dependency tree / graph

Firefox Tracking Flags

(firefox42 affected)



(2 attachments)



3 years ago
Created attachment 8644547 [details]

###!!! ASSERTION: cancel with non-failure status code: 'NS_FAILED(status)', file netwerk/base/nsInputStreamPump.cpp, line 210

Comment 1

3 years ago
Created attachment 8644549 [details]

Comment 2

3 years ago
The bug interestes me, and I investigate its history.

The problem is here:
When the response type is Blob or MozBlob, it cancel the request with NS_OK state. Which cause the assertion.

It's done in Bug 689008. In comment five of that bug, it said that we support "Cancel(NS_OK)". But seems the assertion was exist long before that. I couldn't understand this.

Some more info:
1. No one else did "Cancel(NS_OK)" on an XHR request:

2.Seems that others saw this assertion before, and just ignore it:


3 years ago
Flags: needinfo?(VYV03354)
I think we should just drop support for moz-blob once we've implemented the Streams spec.
Depends on: 1128959
Flags: needinfo?(VYV03354)
Could you check if the assertion is also triggered with "blob" rather than "moz-blob"?
Flags: needinfo?(VYV03354)
Hm, you're right. But I don't know if the assertion is valid or not. Maybe netwerk peers would have more insight.
Flags: needinfo?(VYV03354) → needinfo?(jduell.mcbugs)
My understanding is that necko clients may use "Cancel(NS_OK)" when they want to tell necko that the data transfer could be stopped (and the network connection could be closed, etc), but that the channel was not to be considered a failure.  (in the Blob case it looks like we don't need to serve the actual data immediately?  Or we're getting it from elsewhere?)

I've never been a huge fan of this semantic--it makes the necko code a little tricker (IIRC there are lots of places where we check mStatus and equate !NS_OK with cancelation/failure).  But I'm willing to support it if it's needed. And from a glance at the code in question (now living in XMLHttpRequestMainThread::OnDataAvailable(), it looks like we're only reading from the channel because the nsIStreamListener contract mandates it, and then we cancel.

So my hunch is that we should remove the assertion in nsInputStreamPump::Cancel().  Honza, what do you think?
Flags: needinfo?(jduell.mcbugs) → needinfo?(honzab.moz)
(In reply to Jason Duell [:jduell] (needinfo me) from comment #7)
> So my hunch is that we should remove the assertion in
> nsInputStreamPump::Cancel().  Honza, what do you think?

Hard to say.  But:


And mAsyncStream->CloseWithStatus expects that too.  So, no we can't.

the caller has to migrate to NS_ERROR_SOMETHING and just ignore the error in OnStopRequest, if that is the reason why NS_OK was used.  Apparently, there is no test for this, since it would be perma-crashing on try....

> now living in XMLHttpRequestMainThread::OnDataAvailable() 
Flags: needinfo?(honzab.moz)
I mark this bug as duplicate of bug 1337746. I already wrote a patch for XHR.
Important to know that this assertion is triggered only when the XHR opens a file: URL and the responseType is blob/moz-blob.

About removing moz-blob, I added a telemetry ID in order to know if it's used or not. I'm planning to remove telemetry/moz-blob in 1 release.
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1337746
OK, now that we're not going to support it, should we throw an error for Cancel(NS_OK)?
Flags: needinfo?(honzab.moz)
(In reply to Jason Duell [:jduell] (needinfo me) from comment #10)
> OK, now that we're not going to support it, should we throw an error for
> Cancel(NS_OK)?

"throw an error" - like let Cancel(NS_OK) return some NS_ERROR_FAILURE and do nothing?  Not sure...  maybe add MOZ_ASSERT(NS_FAILED(aStatus)) there (and convert some NS_ASSERTIONS below to it too!) and we are done here.
Flags: needinfo?(honzab.moz)
You need to log in before you can comment on or make changes to this bug.