Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interaction with ServiceWorker #54

Closed
igrigorik opened this issue May 26, 2015 · 25 comments
Closed

Interaction with ServiceWorker #54

igrigorik opened this issue May 26, 2015 · 25 comments

Comments

@igrigorik
Copy link
Member

Has any thought been given to if/when SWs interact with reports? - w3ctag/design-reviews#24 (comment)

@igrigorik
Copy link
Member Author

My first thought is that NEL reports should be transparent to ServiceWorker. The scenarios are:

  • If no SW is installed then every request is routed to HTTP cache > network. If the network fetch fails for a registered NEL origin, the UA queues up an NEL report.
  • If SW is installed, even if the user is offline, the SW can serve a response from local cache for some requests -- in these cases there is no network fetch and no NEL reports. However, if SW forwards the fetch() request to network and then that fails for a registered NEL origin, then the UA would queue up an NEL report:
    • Some requests (e.g. client request + potential client request... iframes, embeds, prerenders) are not seen by SW at all but we still need NEL to fire.
    • For seen requests, the fetch would fail and SW can invoke own logic to generate an appropriate response. The report is an orthogonal feature that's provided by the UA: the UA queues it in the background and delivers without notifying the serviceworker.

Thoughts? /cc @annevk

@annevk
Copy link
Member

annevk commented Jun 9, 2015

We should make this consistent with whatever CSP does. And other specifications that might do reporting?

@igrigorik
Copy link
Member Author

AFAIK, CSP doesn't define anything in this regard: https://w3c.github.io/webappsec/specs/CSP2/#violation-reports

I'd propose that we treat these reporting requests as 'client requests'.

/cc @mikewest @slightlyoff

@annevk
Copy link
Member

annevk commented Jun 10, 2015

Some new type might be better, "report requests" or some such. Note that you can set the "skip service worker flag" yourself, but it might be better to create a logical grouping for this so new report features follow the established pattern.

@annevk
Copy link
Member

annevk commented Jun 14, 2015

According to @ehsan Gecko will run CSP reports through the service worker.

@igrigorik
Copy link
Member Author

Hmm, interesting. I guess for CSP this makes sense since the policy applies to the page you control via SW. As such, you can queue up and dispatch CSP reports once back online, or some such..

For NEL, I think we have different (iframe-like) semantics. For example, consider the following use case: example.com has a SW, and when loaded it requests widget.com/thing, which is a registered NEL origin. If the fetch for widget.com/thing fails, then as per NEL policy it (widget.com) should be notified by the UA; example.com should not intercept this report, the policy belongs to widget.com.

With that in mind, sounds like "skip service worker flag" should do the trick here?

@annevk
Copy link
Member

annevk commented Jun 15, 2015

Well, perhaps if widget.com has a SW, that SW should be notified about the NEL policy. That's what an <iframe> would do anyway.

@annevk
Copy link
Member

annevk commented Jun 15, 2015

Kind of makes sense too. That way widget.com can just collect all kinds of data locally and at some point synchronize with the server.

@slightlyoff
Copy link

+1 to @annevk's point; I'd like to see these reports run through the service worker so that logging can happen offline and be resync'd later.

@igrigorik
Copy link
Member Author

@annevk @slightlyoff my understanding is that if (SW-controlled) example.com embeds widget.com (also SW-controlled) via an iframe, the iframe request will have the skip-sw flag set and it won't automatically spin up the SW for widget.com. Is that not the case? Further, if widget.com load fails, the report is supposed to be delivered to the report-uri registered by widget.com.

Re, happen offline and resync'd later: that's implicit in this case, the UA will take that on already. If you're offline we'll have to persist the report and beacon it later once we can reach the report URI.

@annevk
Copy link
Member

annevk commented Jul 16, 2015

The request for the <iframe> will try to locate its own service worker. It doesn't set the skip-service-worker flag.

@igrigorik
Copy link
Member Author

Ah, interesting, I misunderstood the SW+iframe interaction then. With that in mind, sgtm.

Next question: what's to be done in NEL to account for this? As far as I can tell we don't need to add any additional flags or behaviors in NEL (same story as CSP).

@annevk
Copy link
Member

annevk commented Jul 18, 2015

Yeah, as long as you set all the Fetch parameters correctly everything should be alright.

@igrigorik
Copy link
Member Author

Current logic in processing section:

Queue a task to fetch report URL using HTTP method POST, with a Content-Type header field of application/nel-report, and an entity body consisting of report body. If the origin of report URL is not the same as the origin of the NEL origin for which the report is generated, the block cookies flag must also be set. The task source for these tasks is the Network Error Logging task source.

Anything missing or that needs to be clarified?

@annevk
Copy link
Member

annevk commented Jul 20, 2015

I think you should define it in terms of https://fetch.spec.whatwg.org/#concept-fetch. E.g. defining the actual request you pass to fetch. Defining it in terms of HTML's fetch is not great for those implementing.

@igrigorik
Copy link
Member Author

@annevk any pointers for a good spec example doing that?

@annevk
Copy link
Member

annevk commented Jul 24, 2015

XMLHttpRequest and fetch() do it. You basically do something like

  • Let req be a new request.
  • Set req's method to LALALA.
  • Fetch request.

And cross-reference the terms with Fetch.

@igrigorik
Copy link
Member Author

@annevk thanks, ptal at #55... I'm sure it needs more work, but hopefully it's on the right track.

@igrigorik
Copy link
Member Author

@slightlyoff @annevk independent of the Fetch definition... Thinking out loud:

  • share.com provides a popular share.com/widget.js that many sites embed on the web via a <script> tag, and a share.com/image.jpg that's hot-linked from across the web via <img>.
    • share.com defines an NEL policy to notify share-backup.com/reporting when a fetch failure occurs, such that they can identify and fix when/if these resources are failing.
  • mysite.com installs a SW and intercepts requests to share.com/widget.js and share.com/image.jpg
    • SW does a "passthrough" and attempts to fetch() them from network... which fails.
      • SW's fetch promise is rejected, which allows SW to define some fallback strategy.
      • UA generates an NEL report, which is to be delivered to share-backup.com/reporting.

Q: This NEL report is seen by the mysite.com's SW? It's not clear to me what mysite.com would do with it? It's not its responsibility to deliver that report; share.com was the one that requested the report and its on the UA to deliver it. Shouldn't this report be routed to share.com's SW if one is present, and otherwise not seen by mysite.com?

@annevk
Copy link
Member

annevk commented Jul 26, 2015

Yeah, I guess NEL is different from CSP in that respect. I guess we want NEL to be some kind of client request then. Not sure what the best way would be to model that.

@mikewest, @jungkees, any ideas?

@igrigorik
Copy link
Member Author

@jakearchibald
Copy link

Agree that this should be a client request, given the request can be sent when there's no client open for the origin.

For subresources with redirect-mode "follow" (eg an <img>), if /a redirects to /b and the connection to /b fails, does this count as a failure for /a or /b?

In the same case, what if /a redirects to //other-origin/b and the connection to //other-origin/b fails? Does that report go to //other-origin (if it uses NEL)?

The big question I have is what happens with requests that skip serviceworker. If the report goes to the serviceworker, are we leaking data? It might not be a problem if report always goes to the current request url.

@jungkees
Copy link

I'm trying to get my head around the very purpose of the feature. If it's fine/valuable to expose the reports to script surface, developers can capture the NEL FetchEvent.requests being delivered to share-backup.com/reporting in the registered SW, if any; otherwise NEL may have to reattempt using exponential backoff as-is. Developers may try to send them and if fetch('share-backup.com/reporting') rejects, they should store them (SW Cache can't be used though), and might want to use sync event to batch those stored report requests.

If this is not desirable, we'd have to either add a simpler API (which seems to be not desirable though I haven't gotten through the relevant issues yet) or should not expose this by doing everything within the platform. (We might still be able to define relevant algorithms and the hooks between NEL/Fetch/SW.)

@igrigorik
Copy link
Member Author

@jakearchibald /b and //other-origin would get the error reports, assuming they have valid NEL registrations in place.. If they don't then no NEL report is fired. That is, if request fails against X then we lookup NEL registration for X, and if present deliver the report.

The big question I have is what happens with requests that skip serviceworker. If the report goes to the serviceworker, are we leaking data? It might not be a problem if report always goes to the current request url.

I'm not sure I follow your line of thought here... My understanding is that if we declare NEL reports to be client requests then they bypass all SW logic -- is that not so?

@dcreager
Copy link
Member

I think this issue is now moot:

  • NEL now clarifies that it describes network requests (its own definition), which is effectively "one execution of Fetch's HTTP-network-fetch algorithm". So if a service worker intercepts a request, and prevents the user agent from hitting the network, then no NEL report will be generated.

  • NEL also clarifies that NEL reports are completely invisible to client-side JavaScript code. Fetch does not expose any new information about network errors to its callers, and NEL reports are explicitly not handed off to the new ReportingObserver framework (which is how user-agent-generated reports would be delivered ot client-side code).

I'm going to close this issue; please feel free to re-open if I've misunderstood anything, or if there are other questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants