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

useElectronNet throws error and breaks #443

Closed
mgbennet opened this issue Dec 29, 2017 · 6 comments
Closed

useElectronNet throws error and breaks #443

mgbennet opened this issue Dec 29, 2017 · 6 comments
Labels
bug Something does not work as it should external The issue related to an external project

Comments

@mgbennet
Copy link

When I make a get request with useElectronNet: true within an Electron app, got breaks. Set to true, there is no issue and the request goes through fine. I am using electron 1.7.10 and got 8.0.1.

Running the following code with electron demonstrates my issue.

const electron = require('electron')
const app = electron.app
const got = require('got')

app.on('ready', function() {
    let options = {
        useElectronNet: true
    }
    got.get('https://registry.npmjs.org/-/package/electron/dist-tags', options)
        .then((response) => {
            console.log('got:', response.body);
            app.quit();
        })
        .catch((e) => {
            console.log(e);
            app.quit(); 
        });
});

The error it throws is:

Exception has occurred: TypeError
TypeError: Cannot read property 'once' of undefined
    at EventEmitter.cacheReq.once.ee.once.req ...\node_modules\got\index.js:229:19)
    at Object.onceWrapper (events.js:316:30)
    at emitOne (events.js:120:20)
    at EventEmitter.emit (events.js:210:7)
    at Immediate.cacheReq.once.setImmediate (...\node_modules\got\index.js:264:8)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

The issue is the electron.ClientRequest object at line 224 (or 230 in current master of got/index.js does not have a connection property. This isn't an issue with the http.ClientRequest used when useElectronNet is set to true.

Am I missing something? Is this an issue with Electron? Any insight would be very helpful. Thanks!

@sindresorhus
Copy link
Owner

Can you try #429 and see if it fixes your problems?

Can you also open an issue on Electron about adding the missing connection property?

@sindresorhus sindresorhus added the bug Something does not work as it should label Dec 29, 2017
@mgbennet
Copy link
Author

Using #429, I bypassed the above error but got caught on a second error that I was getting before but didn't mention because I thought it was related to the first.

Exception has occurred: TypeError
TypeError: header.trim is not a function
    at parseCacheControl (c...\node_modules\http-cache-semantics\node4\index.js:24:24)
    at new CachePolicy (...\node_modules\http-cache-semantics\node4\index.js:89:23)
    at ClientRequest.handler (...\node_modules\cacheable-request\src\index.js:62:30)
    at Object.onceWrapper (events.js:316:30)
    at emitOne (events.js:115:13)
    at ClientRequest.emit (events.js:210:7)
    at URLRequest.ClientRequest.urlRequest.on (...\node_modules\electron\dist\resources\electron.asar\browser\api\net.js:207:12)
    at emitOne (events.js:115:13)
    at URLRequest.emit (events.js:210:7)

I'll see about opening an issue on Electron about the missing property.

@mgbennet
Copy link
Author

mgbennet commented Jan 22, 2018

Ok, so #429 did fix the first issue, and with 8.0.2 I get a bit further.

But I still get the second error I mentioned above. This error occurs in http-cache-semantics/index.js, in the function parseCacheControl. With useElectronNet set to false, this function receives a string, With useElectronNet set to true, though, it receives an array instead of length 1, containing the string that should be passed.

This still could be an issue with Electron.net not behaving in the same way as node's http, but I'm getting a bit lost trying to track down where it goes wrong.

Maybe I can fix this with the right options? I'm not sure.

@lukechilds
Copy link
Contributor

lukechilds commented Jan 23, 2018

This still could be an issue with Electron.net not behaving in the same way as node's http, but I'm getting a bit lost trying to track down where it goes wrong.

Yeah, that's the issue.

The parseCacheControl() function is called like this:

parseCacheControl(res.headers['cache-control']);

res.headers should be a "Key-value pairs of header names and values.". If Electron is returning an array for res.headers['cache-control'] then it's behaving completely differently to the built in HTTP client.

In Got 8 we introduced two big features, progress events and caching, that rely on lower level HTTP functionality. We've noticed quite a few problems (#315) with electron.net which is why it's now disabled by default. I'd recommend not setting useElectronNet to true unless you absolutely need it until it has better compatibility with Node.js.

Are you also able to open an issue on Electron about strange header behaviour?

@mgbennet
Copy link
Author

I found it has been reported a year ago in this issue. I've made a note that its affecting version 8.0.0 of got and asked for an update.

However, I did rewind to version 7.1.0 and it works fine. I believe I can live without the features of 8.0.0 for now, so that might be the fix for now if you really need to use Electron.net like I do.

@szmarczak
Copy link
Collaborator

Closing this in favor of #899

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should external The issue related to an external project
Projects
None yet
Development

No branches or pull requests

4 participants