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

Open
mgbennet opened this issue Dec 29, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@mgbennet
Copy link

commented Dec 29, 2017

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

This comment has been minimized.

Copy link
Owner

commented Dec 29, 2017

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 label Dec 29, 2017

@mgbennet

This comment has been minimized.

Copy link
Author

commented Dec 29, 2017

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Collaborator

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

This comment has been minimized.

Copy link
Author

commented Jan 23, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.