977314 - Necko cache returns 200 response for a byte range request when cached response isn't a byte range request

Status: NEW

(Core :: Networking: Cache, defect, P3)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

If we load a resource with a normal HTTP request, the necko cache caches it. Good. But if we later request a 0- byte range request (as the Media code does, see MediaChannelResource) then the cache will use the cache entry for the non-byte range request, and return a 200 response code.

The media code always requests a 0- range as a way to determine whether the server actually supports byte range requests, so returning a 200 code confuses it (our OnStartRequest which does this is here: http://mxr.mozilla.org/mozilla-central/source/content/media/MediaResource.cpp#168 ).


STR:
1. Ensure that content/media/test/test_buffered.html is not doing a 0- byte range request in its XHR (it currently is, I'm going to change it to not to work around this bug).
2. Do an opt build on Windows, do not enable debug.
3. Run the media mochitests:
./mach mochitest-plain content/media/ --start-at content/media/test/test_VideoPlaybackQuality.html --end-at content/media/test/test_bug654550.html --run-until-failure

Expected result: Tests loop for ages
Actual result: timeout in content/media/test/test_bug495300.html.

The timeout is caused by that test trying to seek to the end of file, but because the media stack is served a cached response that is not a 0- byte range, the media stack decides that we're not fully seekable, so the seek-to-the-end winds up seeking to the end of the cached data that's been streamed into the network cache. The bug only happens in opt builds, as debug builds are sufficiently slow enough that the entirety of the media file has streamed into the MediaCache before the test tries to seek the resource to the end, so the end of cached data is the end of the resource, so the test still passes, as the test is waiting for a "reached end of media resource" event.

So we need expose somehow to the media stack that a cached 200 response comes from a server that supports byte range requests, or the resource is fully cached by the necko cache, so if the media stack does a byte range request it can rely on the necko cache satisfying it.

Comment 1

[Patrick McManus [:mcmanus]]

(In reply to Chris Pearce (:cpearce) from comment #0)
> If we load a resource with a normal HTTP request, the necko cache caches it.
> Good. But if we later request a 0- byte range request (as the Media code
> does, see MediaChannelResource) then the cache will use the cache entry for
> the non-byte range request, and return a 200 response code.
> 

Assuming the cache entry is complete, that sounds good.

> The media code always requests a 0- range as a way to determine whether the
>

that's probably not a reasonable assumption :) The 200 is a perfectly fine result code for that reuest (assuming the whole document comes in the response body).. even from the origin or some other proxy on the path even if they do otherwise answer range requests. The 1- range makes more sense as a test (or something else that does not have a 200 equivalent).. but even then servers can change their mind and suddenly start ignoring range and serve up 200's for any particular request so the consumer needs to be aware of that. (I can't figure out if that would be a problem or not right now).

I'm not quite sure what the bug is here.. the cache does not (honza, am I wrong?) generate 206 replies itself- does 626027 cover that?

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Patrick McManus [:mcmanus] from comment #1)
> (In reply to Chris Pearce (:cpearce) from comment #0)
> > If we load a resource with a normal HTTP request, the necko cache caches it.
> > Good. But if we later request a 0- byte range request (as the Media code
> > does, see MediaChannelResource) then the cache will use the cache entry for
> > the non-byte range request, and return a 200 response code.
> > 
> 
> Assuming the cache entry is complete, that sounds good.

That would be fine if we knew that we could otherwise request arbitrary ranges and expect to receive them. But there's no way of telling that at the moment, right?


> > The media code always requests a 0- range as a way to determine whether the
> >
> 
> that's probably not a reasonable assumption :)

Sure, but what can we do instead then?  ;)

The media stack needs to know whether we can reasonably expect to do a Byte Range request to get a non-downloaded segment of the media. Unless we know that, we can't seek a <video>'s playback position into an un-buffered part of the video.


> The 1- range makes more sense as
> a test (or something else that does not have a 200 equivalent).. 

We could do that, though it's a bit ugly. We'd still need an additional request for the first byte too, we need that byte for sniffing sometimes.

 
> I'm not quite sure what the bug is here.. the cache does not (honza, am I
> wrong?) generate 206 replies itself- does 626027 cover that?

If the necko cache generated 206 responses to byte range requests when it had a resource cached, that would probably be ok... But what if the resource was evicted from the cache and the server actually didn't support byte ranges?

Comment 3

[Patrick McManus [:mcmanus]]

> 
> If the necko cache generated 206 responses to byte range requests when it
> had a resource cached, that would probably be ok... But what if the resource
> was evicted from the cache and the server actually didn't support byte
> ranges?

if we accept that the necko cache is standards compliant don't you still have the same problem with any other cache on the path that works the same way? and we of course can't change them..

Comment 4

[Jason Duell]

This is the workaround cpearce landed for the mochitest:

  http://hg.mozilla.org/integration/mozilla-inbound/rev/26bfe4ef1bc2

As discussed on IRC, I think a solution here could be to 1) store a flag in cache entries if they were served as part of a Range response; and 2) add some new load flag that skips the cache unless that flag is set (and replies with a 206 response if it is).  Assuming the server(s) for a given URI don't change in their ability to serve Range requests, this should give us both 1) the ability for media code to know if a URI supports Range, and 2) still gives us the ability to get cache hits for such URIs.

Comment 5

[Patrick McManus [:mcmanus]]

(In reply to Jason Duell (:jduell) from comment #4)
> This is the workaround cpearce landed for the mochitest:
> 
>   http://hg.mozilla.org/integration/mozilla-inbound/rev/26bfe4ef1bc2
> 
> As discussed on IRC, I think a solution here could be to 1) store a flag in
> cache entries if they were served as part of a Range response; and 2) add
> some new load flag that skips the cache unless that flag is set (and replies
> with a 206 response if it is).  Assuming the server(s) for a given URI don't
> change in their ability to serve Range requests, this should give us both 1)
> the ability for media code to know if a URI supports Range, and 2) still
> gives us the ability to get cache hits for such URIs.

jason - this isn't consistent with comment 3. The necko cache isn't the only cache the media code has to work with. Also - that assumption is bogus; its ok to optimize for that case, but things can't break if its wrong (which I think is what this bug report is about - maybe I'm wrong on that?)

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/26bfe4ef1bc2

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Seems like this (severely) re-awoke the intermittent failure in bug 752796.

Comment 8

[Honza Bambas (:mayhemer)]

On IRC we mentioned to expose IsResumable on nsIHttpChannel.  I don't know the code where you recognize the server supports range requests, but the condition is relatively complicated and is already implemented in nsHttpChannel::IsResumable().  Every 200 response that is resumable has Accept-Ranges:	bytes response header.  This header is one of the conditions to allow range requests.  If you are checking for 206 response code as an answer to Range: 0- request, then, sorry, you are doing something wrong.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

We got stung by this again in bug 1151676. This was marked "fixed" when I landed the work around, but it should really have been marked leave-open until an actual fix can be made, so reopening.

Comment 10

[Honza Bambas (:mayhemer)]

Is this only for 0- range requests?  Or should it work for any range?  If the former, then it should be relatively easy to change the response code artificially.  For the letter it's a bit more work but doable.

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

The latter; we check for the 206 response code and Content-Range header in the response every time we do a new HTTP request, and mark the stream as "unseekable" if the server doesn't send either 206 and a valid Content-Range header.

Comment 12

[Honza Bambas (:mayhemer)]

I assume you are using setRequestHeader("Range", "bytes=0-") and not resumeAt().

I'm going to implement this only for 0- range request for now since this is the stated problem in the bug description and also because for any other range we simply bypass the cache at all (for both read and write - see IsSubRangeRequest function).

Comment 13

[Honza Bambas (:mayhemer)]

Attachment: v1

pretty much self-explanatory

https://treeherder.mozilla.org/#/jobs?repo=try&revision=08f9c622270f

Comment 14

[Patrick McManus [:mcmanus]]

Comment on attachment 8714953 [details] [diff] [review]
v1

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

the problem as I read it is that the channel caller wants to know if the origin server supports byte ranges. And it wants to know this based on reusing a cached entry that may not have been made as a range request - which therefore is information we simply don't have in the cache.

It seems to me that the patch turns every cached 200 into a 206 (assuming the request has the right header set). but that doesn't answer the question - does the server support ranges?

As an aside, I think the way to indicate this to the caller channel is probably by the presents of an "accept-ranges: bytes" response header.. and if we get a 206 without one, its fine to inject one and store it with the cache entry.. but that doesn't really get at the original STR which is that some other channel unrelated to the media code has populated the cache without making a byte range request and now the media code would like to both 1] does the origin support ranges, and 2] use the cache entry

I don't think that can be done.. but perhaps if the media code simply knew that the response came from the cache they could exclude it from the "accepts-byte-ranges" algorithm they are using? That sounds like the right thing, because it really doesn't have the information that is wanted without contacting the origin - which kinda defeats the point of the cache.

Comment 15

[Honza Bambas (:mayhemer)]

Yep, that is true.  

Media people claim that just inspecting the response headers is not enough to detect true range support on a server.  They claim the only way is to try with 0- request.  I've already had this discussion with them ;)

Ideally we should:
- when there is a request with 0- range, we should store to the cache entry's metadata the content is result of such range request ; then we can trust the status 206/200 on it
- when another 0- request is made, we check the metadata
- if it says the content is NOT a result of 0-, just reopen the entry (=doom the current, create new one) and go out normally as there were no cached entry before
- otherwise just do what we do now

Makes sense?

Comment 16

[Honza Bambas (:mayhemer)]

Attachment: v2

A better approach:
- we save the info whether the first request was made with 0- in cache entry's metadata
- when another requests is also made with 0- range header, we check the metadata is there
- if yes, we can use the cached content
- if no, we doom the entry and refetch from the network (and cache again)
- tests included

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c56e298c99d

Comment 17

[Honza Bambas (:mayhemer)]

(In reply to Honza Bambas (:mayhemer) from comment #16)
> Created attachment 8715371 [details] [diff] [review]
> v2
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c56e298c99d

So, this is a bit bigger change:


2169 INFO TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_request_context_audio.html | A promise was rejected: [object Event]
2216 INFO TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_request_context_video.html | A promise was rejected: [object Event] 

more details:
 11:35:54     INFO -  833 INFO TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_request_context_audio.html | A promise was rejected: [object Event]
 11:35:54     INFO -      runTest/window.onmessage@dom/workers/test/serviceworkers/test_request_context.js:47:7
 11:35:54     INFO -      EventHandlerNonNull*runTest@dom/workers/test/serviceworkers/test_request_context.js:45:3
 11:35:54     INFO -      Async*onload@dom/workers/test/serviceworkers/test_request_context.js:64:3
 11:35:54     INFO -      EventHandlerNonNull*@dom/workers/test/serviceworkers/test_request_context.js:63:1



I'll need a little help here.  The test is highly generic and I'll need some guidance where to start chasing this error.  Chris?

Comment 18

[Patrick McManus [:mcmanus]]

Comment on attachment 8715371 [details] [diff] [review]
v2

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

I'm still not a fan.. why can't we just tell the caller whether the response came from the cache or not? In the case of a cached response they just shouldn't use it as input to their range algorithm.

Comment 19

[Honza Bambas (:mayhemer)]

(In reply to Patrick McManus [:mcmanus] from comment #18)
> Comment on attachment 8715371 [details] [diff] [review]
> v2
> 
> Review of attachment 8715371 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm still not a fan.. why can't we just tell the caller whether the response
> came from the cache or not? In the case of a cached response they just
> shouldn't use it as input to their range algorithm.

There is the isFromCache property on nsICacheInfoChannel interface.

Chris, would you be OK with using that instead of the hacks I propose in http channel?

Comment 20

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Honza Bambas (:mayhemer) from comment #19)
> (In reply to Patrick McManus [:mcmanus] from comment #18)
> > Comment on attachment 8715371 [details] [diff] [review]
> > v2
> > 
> > Review of attachment 8715371 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I'm still not a fan.. why can't we just tell the caller whether the response
> > came from the cache or not? In the case of a cached response they just
> > shouldn't use it as input to their range algorithm.
> 
> There is the isFromCache property on nsICacheInfoChannel interface.
> 
> Chris, would you be OK with using that instead of the hacks I propose in
> http channel?

What we need to know is whether we can request an arbitrary byte range from the server and expect to get it.

Can we assume that if isFromCache is true that we can request an arbitrary byte range and expect to get it? What if the cache evicts the resource while we're trying to read from it?

If isFromCache allows us to infer that an HTTP Byte Range Request will always succeed, I'd be happy using isFromCache, but it's not obvious to me (not being a networking expert) how isFromCache allows us to infer whether we can expect a byte range request to succeed.

Comment 21

[Patrick McManus [:mcmanus]]

(In reply to Chris Pearce (:cpearce) from comment #20)
> (In reply to Honza Bambas (:mayhemer) from comment #19)
> > (In reply to Patrick McManus [:mcmanus] from comment #18)
> > > Comment on attachment 8715371 [details] [diff] [review]
> > > v2
> > > 
> > > Review of attachment 8715371 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > I'm still not a fan.. why can't we just tell the caller whether the response
> > > came from the cache or not? In the case of a cached response they just
> > > shouldn't use it as input to their range algorithm.
> > 
> > There is the isFromCache property on nsICacheInfoChannel interface.
> > 
> > Chris, would you be OK with using that instead of the hacks I propose in
> > http channel?
> 
> What we need to know is whether we can request an arbitrary byte range from
> the server and expect to get it.
> 

you cannot simultaneously
 a] use data that does not contact the server (i.e. a cache hit). This cached data does not contain the information you are looking for.
AND b] learn about the server's capabilities

I believe your choices are either bypass the cache (i.e. give up A), or just use the data and not infer anything about it for future requests (i.e. give up B). B makes a lot more sense to me - free data; you just don't advance your state machine.

Comment 22

[Honza Bambas (:mayhemer)]

(In reply to PTO until Feb 23 - Chris Pearce (:cpearce) from comment #20)
> (In reply to Honza Bambas (:mayhemer) from comment #19)
> > (In reply to Patrick McManus [:mcmanus] from comment #18)
> > > Comment on attachment 8715371 [details] [diff] [review]
> > > v2
> > > 
> > > Review of attachment 8715371 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > I'm still not a fan.. why can't we just tell the caller whether the response
> > > came from the cache or not? In the case of a cached response they just
> > > shouldn't use it as input to their range algorithm.
> > 
> > There is the isFromCache property on nsICacheInfoChannel interface.
> > 
> > Chris, would you be OK with using that instead of the hacks I propose in
> > http channel?
> 
> What we need to know is whether we can request an arbitrary byte range from
> the server and expect to get it.
> 
> Can we assume that if isFromCache is true that we can request an arbitrary
> byte range and expect to get it?

Not easy to answer.  If we have the whole content cached (which is unlikely in case of larger (50MB+) media) then you will probably never hit the server.  But problem is that we don't serve subranges from the cache.  Any N- range for N>0 will go to the server regardless the cache state at all.

So, unless the cached response is a result of 0- response, you don't know anything.  And if it is result of 0- and the response from the server is 200, you know nothing as well.  That however the patch v2 solves for you.

>  What if the cache evicts the resource while
> we're trying to read from it?

If the eviction happens after you open the http channel, then you can read it.  Such entry is not freed until last reference to it is released.

> 
> If isFromCache allows us to infer that an HTTP Byte Range Request will
> always succeed, I'd be happy using isFromCache, but it's not obvious to me
> (not being a networking expert) how isFromCache allows us to infer whether
> we can expect a byte range request to succeed.

isFromCache only tells you the content you are getting from the channel is coming from the cache.  Nothing more.

(In reply to Patrick McManus [:mcmanus] from comment #21)
> > What we need to know is whether we can request an arbitrary byte range from
> > the server and expect to get it.
> > 
> 
> you cannot simultaneously
>  a] use data that does not contact the server (i.e. a cache hit). This
> cached data does not contain the information you are looking for.
> AND b] learn about the server's capabilities
> 
> I believe your choices are either bypass the cache (i.e. give up A), or just
> use the data and not infer anything about it for future requests (i.e. give
> up B). B makes a lot more sense to me - free data; you just don't advance
> your state machine.


They want range capability determination from the very start to know whether to allow seeking.  It's a bad UX to let user click on the search bar and then either nothing or something unexpected (like restart of the video from the beginning) happens.

I think the patch v2 gives the behavior needed.


Chris:
- how often are you hitting this bug?  
- in general, how often are you ever reading data from the cache?


If it's low rate, I would suggest to just bypass caching at all (LOAD_BYPASS_LOCAL_CACHE | LOAD_INHIBIT_CACHING).  The easiest solution.

Comment 23

[Chris Pearce [:cpearce (Not reading bugmail)]]

We will hit this bug in the wild if we load a media resource and then the media cache (which is separate from the necko cache) evicts the start of the resource, and then the user seeks to the start of the resource and tries to play there. We'd need to be under mediacache pressure in order to hit this, and for that we have 500mb of available space to play with.

We could also hit this if something else downloads the same file we're playing without the 0- range request, which is a situation I think would be uncommon outside our mochitests.

We'd hit this a lot in our media mochitests if we didn't work around the issue by having ^headers^ on all our media files with "Cache-Control:no-store" on them. That's because we test with a reduced-size media cache to ensure it's pressured.

In the wild, I don't know how common this actually is.

I'd like to keep using the Necko cache as the situation where I'd like to benefit from it is when loading and reloading a lot of small files.

Maybe the best thing to do here is to add telemetry to count what proportion of media 0- requests have isFromCache==true/responseCode==200, and what proportion of those don't have "Accept-Ranges:bytes", with the intention of proving that passing (LOAD_BYPASS_LOCAL_CACHE | LOAD_INHIBIT_CACHING) doesn't hurt too much. And proving whether how reliable "Accept-Ranges:bytes" is these days.

Maybe we can show that it's safe to change the media cache's assumption of seekableness logic to (responseCode==206 || response headers contains "Accept-Ranges:bytes")?

Comment 24

[Honza Bambas (:mayhemer)]

(In reply to Chris Pearce (:cpearce) from comment #23)
> Maybe we can show that it's safe to change the media cache's assumption of
> seekableness logic to (responseCode==206 || response headers contains
> "Accept-Ranges:bytes")?

You or someone had strong voices claiming that range request + 206 response was the only reliable way.  Maybe that's true, but we have no prove for either yes or no.

I presume you are aware that for request Range: N- where N>0 we bypass and inhibit the cache completely?  Hence your desire to use this only for HTTP caching small files, right?  Also remember that anything >50MB is not cached at all, even intermittently.

I think to be optimal here, my patch v2 is the way to go.

Comment 25

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Honza Bambas (:mayhemer) from comment #24)
> (In reply to Chris Pearce (:cpearce) from comment #23)
> > Maybe we can show that it's safe to change the media cache's assumption of
> > seekableness logic to (responseCode==206 || response headers contains
> > "Accept-Ranges:bytes")?
> 
> You or someone had strong voices claiming that range request + 206 response
> was the only reliable way.  Maybe that's true, but we have no prove for
> either yes or no.

Yes, that was me. Some years ago when the media code was young, we encountered a number of big sites that didn't implement the HTTP 1.1 Byte Range Request spec correctly. Amazon S3 was a notable offender, though they've since fixed this - but only for resources added after about 2009 IIRC.

I suggested adding telemetry so that we can prove whether this is still a problem that we need to worry about. I am happy to change my position when shown proof my position is wrong.

 
> I presume you are aware that for request Range: N- where N>0 we bypass and
> inhibit the cache completely?  Hence your desire to use this only for HTTP
> caching small files, right?  Also remember that anything >50MB is not cached
> at all, even intermittently.

Yes, this is all fine. I'm mostly concerned about making sure small files are cached by Necko.

We have our own cache that helps with the large file case, but unlike the Necko cache it evicts its data as soon as the last video/audio element corresponding to a URI is destroyed. I'd also expect that large files aren't reloaded or shared amongst audio/video elements a lot, whereas small files I'd expect are. 


> I think to be optimal here, my patch v2 is the way to go.

Just so I understand the behaviour of your v2 patch, if we make a 0- request, and the cached value wasn't for a 0- request, we'll redownload the resource using a 0- request so that the response code will be (hopefully be) 206? I think that would be fine.

Comment 26

[Honza Bambas (:mayhemer)]

(In reply to Chris Pearce (:cpearce) from comment #25)
> I suggested adding telemetry so that we can prove whether this is still a
> problem that we need to worry about. I am happy to change my position when
> shown proof my position is wrong.

Go for it!

> Just so I understand the behaviour of your v2 patch, if we make a 0-
> request, and the cached value wasn't for a 0- request, we'll redownload the
> resource using a 0- request so that the response code will be (hopefully be)
> 206?

Yes, exactly.  The patch actually makes the channel behave a way you can believe the response code.

Comment 27

[Honza Bambas (:mayhemer)]

Comment on attachment 8715371 [details] [diff] [review]
v2

Patrick, please reconsider this patch based on the discussion.  If you don't like it, or feel the fix doesn't belong to necko at all, I'll close this as WONTFIX.

Comment 28

[Patrick McManus [:mcmanus]]

Comment on attachment 8715371 [details] [diff] [review]
v2

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

ok. I don't really think this is going to be 100% accurate - but the implementation is fine and I'm willing to try. Sorry for the delay.

Comment 29

[Honza Bambas (:mayhemer)]

Attachment: v2 (don't reuse cached 200 response on 0- request) [rebased, updated ci message]

Comment 30

[Honza Bambas (:mayhemer)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4d355b7f9a0707daa747ff290ba28e9ebcf7def

Comment 31

[Honza Bambas (:mayhemer)]

Try from comment 16 is apparently orange.  The one from comment 30 has other build failing patches in the series.  This needs some more attention.

Comment 32

[Honza Bambas (:mayhemer)]

The try from comment 30 failures are:

M(4)
714 INFO TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_request_context_audio.html | A promise was rejected: [object Event]

761 INFO TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_request_context_video.html | A promise was rejected: [object Event] 

W(5)
TEST-UNEXPECTED-FAIL | /service-workers/service-worker/fetch-request-redirect.https.html | Verify redirect mode of Fetch API and ServiceWorker FetchEvent. - promise_test: Unhandled rejection with value: object "Error: Loading redirected audio with Request.redirect=fol..."

TEST-UNEXPECTED-FAIL | /service-workers/service-worker/fetch-request-redirect.https.html | Verify redirected of Response(Fetch API) and ServiceWorker FetchEvent. - promise_test: Unhandled rejection with value: object "Error: wait_for_state must be passed a ServiceWorker"

TEST-UNEXPECTED-FAIL | /service-workers/service-worker/fetch-request-redirect.https.html | Verify redirected of Response(Fetch API), Cache API and ServiceWorker FetchEvent. - promise_test: Unhandled rejection with value: object "Error: wait_for_state must be passed a ServiceWorker"


Repushed to figure out what is still failing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=df6eb4c9d6ea

Comment 33

[Honza Bambas (:mayhemer)]

Pushed again to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c54bd70e6921

I can't reproduce the failures locally.

Comment 34

[Honza Bambas (:mayhemer)]

Ehsan, Ben, I may ask for some help here.  dom/workers/test/serviceworkers/test_request_context_audio.html is failing (see comment 32 and try run in comment 33) with this patch.  

The test is one of the 'super generic' tests that I absolutely don't see in.  Can you please help how to diagnose at least those service worker failures?  Thanks.

Comment 35

[Ben Kelly [:bkelly, not reviewing]]

So I see this stuff in the log:

 04:22:34     INFO -  JavaScript error: , line 0: uncaught exception: http://mochi.test:8888/tests/dom/workers/test/serviceworkers/fetch/context/index.html?testAudio
 04:22:34     INFO -  JavaScript warning: http://mochi.test:8888/tests/dom/workers/test/serviceworkers/fetch/context/index.html?testAudio, line 125: unreachable code after return statement
 04:22:34     INFO -  JavaScript warning: http://mochi.test:8888/tests/dom/workers/test/serviceworkers/fetch/context/index.html?testAudio, line 144: unreachable code after return statement
 04:22:34     INFO -  JavaScript warning: http://mochi.test:8888/tests/dom/workers/test/serviceworkers/fetch/context/index.html?testAudio, line 242: unreachable code after return statement
 04:22:34     INFO -  JavaScript error: , line 0: uncaught exception: http://mochi.test:8888/tests/dom/workers/test/serviceworkers/fetch/context/audio.ogg
 04:22:35     INFO -  696 INFO TEST-PASS | dom/workers/test/serviceworkers/test_request_context_audio.html | The active worker should be available.
04:22:35 INFO - 697 INFO TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_request_context_audio.html | A promise was rejected: [object Event] 

From:

https://treeherder.mozilla.org/logviewer.html#?job_id=26018822&repo=try#L6948

I think you should add debug statement to your try build to see what is failing in there.  At a guess, the audio element is firing onerror here:

https://dxr.mozilla.org/mozilla-central/source/dom/workers/test/serviceworkers/fetch/context/index.html#77

Comment 36

[Honza Bambas (:mayhemer)]

I haven't touched this a long time.  If there is still a strong desire for fixing this bug, feel free to assign back to me or ping :jason to find an owner.

Comment 37

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 38

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258