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

this.pingIntervalTimer.refresh is not a function #631

Closed
JeffWinder opened this issue Dec 1, 2021 · 8 comments
Closed

this.pingIntervalTimer.refresh is not a function #631

JeffWinder opened this issue Dec 1, 2021 · 8 comments
Labels
bug Something isn't working

Comments

@JeffWinder
Copy link
Contributor

JeffWinder commented Dec 1, 2021

Describe the bug
socket.js:108 Uncaught TypeError: this.pingIntervalTimer.refresh is not a function
at Socket.onPacket (socket.js:108)

To Reproduce
engine.io: 6.1.0
engine.io-client: 6.1.1
socket.io: 4.4.0
socket.io-client: 4.4.0

Platform:

  • OS: windows 10

Additional context
Adding
console.log(typeof this.pingIntervalTimer)
just before line 108 shows that it's a number, not a Timer.

this.pingIntervalTime.refresh()
was added here it seems:
37474c7

@JeffWinder JeffWinder added the bug Something isn't working label Dec 1, 2021
@darrachequesne
Copy link
Member

Which version of Node.js are you using? The refresh method was added in v10.2.0.

@JeffWinder
Copy link
Contributor Author

JeffWinder commented Dec 1, 2021

Yes I saw that: https://nodejs.org/api/timers.html#timeoutrefresh
I've tried with 16.9.1, 16.11.1 and 16.13.0

@JeffWinder
Copy link
Contributor Author

And now I'm suddenly getting the type of Timer back instead of number, for all three node versions mentioned above, no idea what changed on my end. As if it was seen as the javascriptsetTimout before. So now the error is gone.

@JeffWinder
Copy link
Contributor Author

JeffWinder commented Dec 2, 2021

Okay, I think I see what happened. When I wanted to console.log setTimeout in socket.js, VS Code added
const { setTimeout } = require('timers'); for me when I typed it.
This makes the error go away. So it looks like when leaving out this import javascript's setTimeout is being used, resulting in the error?

@darrachequesne
Copy link
Member

Were you able to find a solution? Is there something we can fix on our end?

@JeffWinder
Copy link
Contributor Author

I think it would be best to explicitly import setTimeout and clearTimeout from Node's timers module, by adding this in lib/socket.ts:
import { setTimeout, clearTimeout } from "timers";
That would make sure compilers don't mistaken those functions for javascript timers, of which setTimeout doesn't have a refresh function. Hence the error in my scenario.

I can also make a PR for that tomorrow if you like.

@JeffWinder
Copy link
Contributor Author

Here's the PR: #632

@darrachequesne
Copy link
Member

Closed by #632.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants