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

Bug: devtools reload-and-profile feature is defeated by sync-xhr feature policy #20806

Closed
roippi opened this issue Feb 11, 2021 · 5 comments · Fixed by #20879
Closed

Bug: devtools reload-and-profile feature is defeated by sync-xhr feature policy #20806

roippi opened this issue Feb 11, 2021 · 5 comments · Fixed by #20879

Comments

@roippi
Copy link

roippi commented Feb 11, 2021

React version: all

Steps To Reproduce

  1. Visit a site that uses Feature-Policy: sync-xhr 'none' and has the profiling build of react enabled
  2. Attempt to use the "reload and start profiling" feature of devtools
  3. The xhr request will fail on reload because it attempts to make a synchronous XHR call. The profiling tab will be stuck in "press record to stop recording" state. Devtools will now fail to load on all subsequent refreshes unless the developer clears session storage, because it keeps trying and failing to make that XHR request.

The current behavior

see 3. above

The expected behavior

Ideally, this feature would not depend on synchronous XHR to function. Browsers are deprecating it and sites are blocking it via Feature-Policy due to third-party javascript abusing it.
In its current state, the feature should fail more gracefully when the XHR is blocked due to feature policy or in the future when browsers start blocking it by default.

@roippi roippi added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Feb 11, 2021
@bvaughn
Copy link
Contributor

bvaughn commented Feb 12, 2021

I'm not sure if that deprecation will apply to extensions. Hm. Regardless, I don't know of any other technique available to implement the reload-and-profile behavior than the sync XHR approach we currently use. (I discussed this approach with Chrome extension dev rels before implementing it because I was concerned about it too.)

Any interest in submitting a PR that detects Feature-Policy: sync-xhr 'none' and disables the reload-and-profile button with a tooltip/message explaining why? :)

@bvaughn bvaughn added Component: Developer Tools Type: Discussion and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Feb 12, 2021
@roippi
Copy link
Author

roippi commented Feb 12, 2021

I'll reach out to my dev rel folks too to see if there's any alternative. Regardless yeah reworking the feature is dependant on finding that better alternative so we can put that part of the issue aside.

I should have the time/ability to submit a PR for the ensafeining. If I don't, and some enterprising individual finds this in the meantime, the way to introspect this in JS is:

document.featurePolicy.allowsFeature('sync-xhr')

@ChrisDobby
Copy link
Contributor

Not sure if I count as an enterprising individual but thought I'd have a go at this!

@bvaughn
Copy link
Contributor

bvaughn commented Mar 11, 2021

Thanks Chris! 👍🏼

@roippi
Copy link
Author

roippi commented Mar 11, 2021

Thanks Chris et al! I'll follow up with a new issue if I find an alternative for sync xhr.

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

Successfully merging a pull request may close this issue.

3 participants