setTimeout().unref() is not a function

[bpasero]

Seeing in shared process:

bootstrap-window.js:253 TypeError: setTimeout(...).unref is not a function
    at IncomingMessage.<anonymous> (Sender.ts:138)
    at IncomingMessage.emit (events.js:327)
    at endReadableNT (internal/streams/readable.js:1327)
    at processTicksAndRejections (internal/process/task_queues.js:80)

Originates from Sender.ts of app insights it seems.

https://ticino.blob.core.windows.net/sourcemaps/753319a08f8e74700b1dba3a29bf0a6af7c2953f/node_modules/applicationinsights/Library/Sender.ts

[lramos15]

Hmmm, unref has been around in node for awhile. Are you doing anything special? It seems like a lot of the errors with unref happen when you accidentally execute in a browser based context rather than node.

[bpasero]

Nothing special afaik, just show the shared process window via F1:

Keep in mind that the shared process is a browser window, so setTimeout does not return a function but a number!

[bpasero]

I still see this in exploration builds but not insiders 🤷

[lramos15]

Is there something special about exploration builds? I just added a try catch

[bpasero]

@lramos15 yeah maybe how uncaught exceptions are handled and reported. This one you cannot really catch as it happens from an event handler:

[lramos15]

Yeah like I said not sure what we should do here as one module expects node and another module expects window and the shared process is a bit in between

[bpasero]

This is still happening.

Btw found microsoft/ApplicationInsights-node.js#367. Maybe we need to contribute a PR.

[lramos15]

Is the issue saying we're trying to use the node SDK in the electron renderer?

[bpasero]

Yeah in the renderer we get the setTimeout variant from DOM and not node.js. I was actually looking into whether we could simply shim the unref() method, but it didn't seem to easy, unless we would overwrite the entire setTimeout method for everyone.

[lramos15]

Maybe we should be using the web SDK in the renderer then? We use the web telemetry reporter in the /browser/ telemetryService so I'm surprised that we're using the node one in the renderer.

[bpasero]

Yeah that sounds reasonable. Though the fact that the shared process is a browser window is a bit of a debt issue, eventually we probably want to go away from running it as hidden window and use a pure node.js process once #131798 landed.

Given that, maybe we should just ignore the issue for now until we get to a real node.js process.

[lramos15]

Going to reopen this since it hasn't really been fixed and Daniel is seeing the issue as well. We are working on switching modules so maybe that will help but still want to keep this as a tracking issue.

[lramos15]

Closing this as we have switched modules