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

Fixes the issue with scalar executing. #65

Closed
wants to merge 6 commits into from

Conversation

mikroware
Copy link
Contributor

The result variable in the callback is in most cases a valid empty array, which results tests as true in a statement. Thus, need to test array validity and length above 0.

Should fix issues #62 and #63

@ghost
Copy link

ghost commented May 4, 2019

I've used @PichotM version of the fix, I am not sure about the keepAlive issue, how and why it appears.

Is it mysql only? Is it only with xampp only?

I never experienced it with MariaDB.

@mikroware
Copy link
Contributor Author

Yes sorry, those were accidental personal commits. The essential was the extra statement below which might be cleaner then PichotM's solution (which assumes it is an array and simply does a javascript true check on the first element).

 && Array.isArray(result) && result.length > 0

@ghost
Copy link

ghost commented May 4, 2019

Well there are some people having issues with the connection timing out on possibly a pool, so I am not certain in how far it is needed or for which mysql versions it is needed, as I never encountered it. So I have to consider adding this with a setInterval though, instead of repeated setTimeout.

For your argument, well...
It looks cleaner, but I am not certain if it covers all the replies from mysql.js
edit: I think PichotMs would return undefined, thus false if it fails.

I could not really work on FiveM for months, and I hope, I can get some test servers running for me later today, so I can test stuff again.

@mikroware
Copy link
Contributor Author

We also seem to have that connection timeout problem. Which actually was the reason why I added the keepAlive function in my fork (which was not supposed to end up here) in order to test this. I have not recently heard any problems related to the timeout error from my development team though.

Together with the keep alive I also updated the mysql package to the most recent version. So I am not sure what change might have probably fixed the problem.

I use the mysql package in a nodejs project (also older versions) with its connection pooling. There I have not had any trouble with connection timeouts though.

@ghost
Copy link

ghost commented May 5, 2019

2.16 already bled memory on linux, although that was months ago, maybe that changed.

@ghost
Copy link

ghost commented May 5, 2019

Would you mind doing a proper pr without compiled code for the next release next weekend with the keepAlive in setInterval?

@ghost ghost closed this May 5, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant