sindresorhus/got

useElectronNet throws error and breaks

mgbennet opened this issue · 6 comments

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!

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?

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.

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.

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?

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.

Closing this in favor of #899