Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Doesn't work inside a service-worker on Firefox #2857

Closed
koalalorenzo opened this issue Sep 1, 2018 · 22 comments
Closed

Doesn't work inside a service-worker on Firefox #2857

koalalorenzo opened this issue Sep 1, 2018 · 22 comments
Assignees
Labels
exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) kind/resolved-in-helia pkg:http-client Issues related only to ipfs-http-client

Comments

@koalalorenzo
Copy link
Member

By playing around with js-ipfs-api inside a service worker, I have noticed that it works perfectly on Safari and Chrome, but apparently not on Firefox.

The error reported is related to the usage of XMLHTTPRequest.

I guess this is the cause: https://www.fxsitecompat.com/en-CA/docs/2015/xmlhttprequest-is-no-longer-available-in-service-workers/

Any guess how to solve/fix this to support also Firefox service workers?

@vasco-santos
Copy link
Member

That is correct, XMLHTTPRequest is not supported anymore on service workers.

Accordingly, XMLHTTPRequest needs to be changed and the fetch API seems like the best option.

We fixed a XMLHTTPRequest in js-ipfs js-ipfs#1478. Would you like to create a PR for solving this here?

@koalalorenzo
Copy link
Member Author

koalalorenzo commented Sep 2, 2018

I will try to find the time for it 👍

Should I update it for all the requests? I was diving a little and I found this line:
https://github.com/ipfs/js-ipfs-api/blob/231c4d78723d41152e6a695a5f171feae2274c19/src/utils/send-request.js#L174

That connects to:
https://github.com/ipfs/js-ipfs-api/blob/231c4d78723d41152e6a695a5f171feae2274c19/src/utils/request.js#L1-L12
https://github.com/ipfs/js-ipfs-api/blob/master/src/utils/request.js

I see from the code that there is a lot of dependency on how we are using this module. I guess I need to change that... but my bet is that would complicate a lot of things since changing it would mean to change the way we set up headers, the requests etc etc etc.

In any case can you guide a little bit me to find what would be the best solution? Maybe also @alanshaw can give me some suggestion a la planning 😃

@vasco-santos
Copy link
Member

Nice research!

I believe that we should have fetch for every single request, since each api call should be accessible through a service worker as well.

Yes, I suppose it will not be an immediate and easy change, as you say, some of the request's building blocks have to be implemented according to the fetch API. Let's wait for @alanshaw input here.

@hugomrdias
Copy link
Member

In the browser we use stream-http package that is suppose to use fetch if it's available we should report this problem to them

@shunkino
Copy link
Contributor

HI @koalalorenzo, I think this is because ReadableStream() is disabled in Firefox by default.
This function should be available for stream-http to work as a fetch() mode.

Could you test your code after enabling ReadableStream() by procedure below?

https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream

From version 57: this feature is behind the dom.streams.enabled preference (needs to be set to true) and the javascript.options.streams preference (needs to be set to true). To change preferences in Firefox, visit about:config.

@koalalorenzo
Copy link
Member Author

The error that I am getting when I am doing a simple list of pinned elements is reporting a XMLHTTPRequest so I am not sure how much that is related to ReadableStream, but I could be wrong.

screen shot 2018-09-10 at 09 06 47

https://www.fxsitecompat.com/en-CA/docs/2015/xmlhttprequest-is-no-longer-available-in-service-workers/

@koalalorenzo
Copy link
Member Author

koalalorenzo commented Sep 10, 2018

Enabling ReadableStream on Firefox it doesn't fix the problem. I think it is more related to the code that I was reporting, but again I am not 100% sure.

@hugomrdias
Copy link
Member

I'm looking into stream-http for other stuff I'll check this out.

@jhiesey
Copy link

jhiesey commented Sep 10, 2018

@hugomrdias let me know if you need help with stream-http. I'm the maintainer of that module and also working for Protocol Labs.

@koalalorenzo
Copy link
Member Author

Can anybody explain to me why it is stream-http the cause of this?

@shunkino
Copy link
Contributor

shunkino commented Sep 10, 2018

@koalalorenzo
It's because js-ipfs-api uses stream-http as an alternative of http in browser environment.
https://github.com/ipfs/js-ipfs-api/blob/master/package.json#L11

@shunkino
Copy link
Contributor

@koalalorenzo
Hi again, sorry for sending comments frequently. Thank you for testing my previous post. Sorry to hear that it didn't worked out for you. 😢

I just tested ServiceWorker inside Firefox and was able to call ipfs.pin.ls() from it. My environment: Firefox (Quantum 61.0.2 on MacOS). Only change I did is enabling ReadableStreaming()

This is the screenshot of my console.
image
Sorry for random logs, I was in hurry...

From the result, I think if stream-http run in the fetch mode, ipfs.pin.ls() functions can be called from ServiceWorker. Could you set break point here and check what mode stream-http is running in your Firefox?

https://github.com/jhiesey/stream-http/blob/718f5127ec970e91ea4c323c2491efdffb22a1c9/lib/request.js#L10

Hope this helps. 😄

@koalalorenzo
Copy link
Member Author

Well, I can't ask the users to enable ReadableStreaming :(
@jhiesey maybe you can help here

@lidel
Copy link
Member

lidel commented Sep 17, 2018

FYSA Streaming API is very close to be enabled by default in Firefox:

[..] the only blocker (or at least the main one I know of) is bug 1385890 and Jason's working on it. Hoping to ship soon.
[meta] enable streams API by default

cc ipfs/in-web-browsers#55

@alanshaw
Copy link
Member

@koalalorenzo this should now be fixed in ipfs-api@25.0.0 by ipfs-inactive/js-ipfs-http-client#868 please open a new issue if the problem persists

@hugomrdias hugomrdias self-assigned this Oct 15, 2018
@hugomrdias
Copy link
Member

@alanshaw this is related to stream-http going xhr mode instead fetch mode when it doesn't detect web stream support, and sw doesn't have xhr. I'm gonna reopen and assign it to me.

@hugomrdias hugomrdias reopened this Oct 15, 2018
@alanshaw
Copy link
Member

argh, yes sorry, thanks @hugomrdias

@koalalorenzo
Copy link
Member Author

Any update?

@lidel
Copy link
Member

lidel commented Dec 9, 2018

FYSA streams just got enabled in Firefox Nightly 👀 🎉 :

2018-12-09--20-54-05

Refs.

@lidel
Copy link
Member

lidel commented Jan 29, 2019

aaaand Firefox 65 just landed in stable channel with support for ReadableStream 🎉

2019-01-30--00-36-04

@koalalorenzo are you able to re-check with Firefox 65?

@koalalorenzo
Copy link
Member Author

I am not able to test again as we decided to implement it in a different way in order to unlock what we were doing on Orion. Sorry :(

@achingbrain achingbrain transferred this issue from ipfs-inactive/js-ipfs-http-client Mar 9, 2020
@achingbrain achingbrain added kind/bug A bug in existing code (including security flaws) exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue pkg:http-client Issues related only to ipfs-http-client labels Mar 9, 2020
@whizzzkid
Copy link

whizzzkid commented May 31, 2023

js-ipfs is being deprecated in favor of Helia. You can follow the migration plan here #4336 and read the migration guide.

This issue has been resolved in Helia! if this does not address your concern please let us know by reopening this issue before 2023-06-05!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/wizard Extensive knowledge (implications, ramifications) required help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) kind/resolved-in-helia pkg:http-client Issues related only to ipfs-http-client
Projects
No open projects
Status: Done
Development

No branches or pull requests

9 participants