wzhouwzhou / ytsearcher

YTSearcher | 170k+ DL | 1000+ Dependents | NodeJS package providing an easy-to-use promise-based solution for getting youtube search results.
https://ytsearcher.willyz.cf
Apache License 2.0
18 stars 2 forks source link

Crashing Silently [Exit Status 1] #2

Closed BannerBomb closed 6 years ago

BannerBomb commented 6 years ago

I been messing with the package because I use it for my discord bot and before updating the package it didn't crash. my command I used it for gets information from a youtube video/current streaming video and it worked until I updated the package. So I decided to mess with the package to see if I can find what part was causing it to crash and anytime you call publishedAt or just [var].first it crashes silently with a exit status 1. Here is a code sample I was working with to test maybe it will help explain better than me.

const { YTSearcher } = require('ytsearcher');
const search = new YTSearcher({
    key: process.env.YOUTUBE_API_KEY,
    revealkey: true
});
search.search("https://www.youtube.com/watch?v=FVovq9TGBw0", { type: 'video' }).then(searchResult => {
    let result = searchResult.first;
    console.log(result.id); // This works just fine
    console.log(result.url); // This works just fine
    console.log(result.publishedAt); // this crashes silently
    console.log(result); // this will also crash silently
}).catch(err => console.error(`ERROR CATCH: ${err.toString()}`));
BannerBomb commented 6 years ago

Sorry, I couldn't edit my issue above because github doesn't like supporting windows vista anymore and that's what I am stuck with rn. But as I stated above: console.log(result.publishedAt); crashes silently but let udate = new Date(result.publishedAt).getTime(); console.log(udate); works just fine without crashing.

BannerBomb commented 6 years ago

again I can't edit anything once I post it because I am on a old computer, but just an update the only 2 things I found that is making it silently crash is result.description and result.publishedAt so like

console.log(result.description); // crashes
console.log(result.publishedAt); // crashes
let udate = new Date(result.publishedAt).getTime(); // this will NOT crash
console.log(udate); // does NOT crash

sorry for all the comments just trying to keep you updated on this. This will be the last comment.

wzhouwzhou commented 6 years ago

Good afternoon @BannerBomb, thank you for opening this issue.

I am referencing some standard preliminary checks as protocol, just to confirm the package version is the latest 1.1.3: *nix/darwin npm list | grep ytsearcher windows npm list | find ytsearcher powershell npm list | sls ytsearcher ... or any other way you use to check the version of the installed package. If the version isn't 1.1.3, run npm i -S ytsearcher@1.1.3 to update to 1.1.3

I was wondering what node version you were running this on? I have tested the following in three separate environments and node versions: (Darwin 17.7.0 on macOS High Sierra 10.13.6) v10.8.0, (Ubuntu 16.04.3 LTS (GNU/Linux 4.4.0-130-generic x86_64)) v8.4.0, and (Docker Linux 4.9.93-linuxkit-aufs) v8.11.3. I have further tested using repl-like eval via a beta version of Chipsbot (my Discord bot) running on the Docker instance. All have resulted in no errors (everything logs as it should, exit code is logged as 0 for v10 and v8.4.0 via directly calling node /path/to/tester.js on the file, and no crashes on the beta bot):

const { YTSearcher } = require('ytsearcher');
const search = new YTSearcher({
  key: '[redacted]',
  revealkey: true,
});
search.search('https://www.youtube.com/watch?v=FVovq9TGBw0', { type: 'video' }).then(searchResult => {
  let result = searchResult.first;
  console.log(result.id);
  console.log(result.url);
  console.log(result.publishedAt);
  console.log(result);
  console.log(result.description);
  let udate = new Date(result.publishedAt).getTime();
  console.log(udate);
}).catch(err => console.error(`ERROR CATCH: ${err.toString()}`));

process.on('exit', code => console.log(`Exit code ${code}`));

Please try running the above as a standalone file in a new workspace or environment if you have not already done so to be sure that the problem lies within the package itself.

Unfortunately I am not aware of any possible causes as to why the package would cause your vm to silently exit. Most properties of a search result are either copied directly from the payload from google or wrapped in some way (i.e. publishedAt comes in as a string but is wrapped to contain a Date).

Thanks again for trying to work this out with me.

~ William

BannerBomb commented 6 years ago

Here is some of the main files if you would like to setup the same environment. And if you need me to record a video of my eval command executing the parts that crash the bot and the parts that work fine. Let me know what I can do if you still need more information about the issue.

package.json nowplaying.js this file I took out the problem code nowplaying.js this file I kept the problem lines and added comments beside the lines that silently crash. globals.js

I had to switch from using node to boot my bot and use nodemon, lint, husky, and yarn to start it up through a binary. Because with node my bot became completely broken and wouldn't start up anymore.

BannerBomb commented 6 years ago

I looked back at my code and for my on 'exit' process I only have it set to shut the bot down. Would that be why it only exits as exit status 1 with no error? process.on('exit', () => this.shutdown());

wzhouwzhou commented 6 years ago

I checked your package json and it appears that the version is correct.

Regarding how you have to start up your bot through a binary, perhaps whatever compiler you are using is having issues compiling ytsearcher.

Run Time Dependency Analysis (RTDA): Nodemon: Autorestarter for node.js applications, as such, despite not being familiar with the source, I would assume it calls upon a node.js binary to run processes. Lint: Code linting in general helps prevent against typos, control style, among other benefits. However, these modifications are (usually) done during the development of the code, not during run time. Thus I believe lint would be unrelated to the current issue (unless whatever linter you are using is aggressively and incorrectly fixing problems for you). Husky: Management of git hooks; this would help for your personal development with regards to git hooks such as pushing to a repo but still this should be unrelated to run time issues. Yarn: Package manager (alternative to npm). Yarn would help in terms of adding the ytsearcher package to your project dependencies but I believe would otherwise have no effect on run time issues either.

I'm slightly confused about what you mean by compiling your bot down to a binary. Would you mind showing the output of running node -v in a shell? I am suspecting that it has to do with the fact that you might be using Windows Vista and the node version is outdated, or something in my package is not being compiled correctly. It seems that anything in the RTDA should not be the cause of this. To be sure, also try to take my test file code from above, save it to a file, and run it before and after compiling it. If an error occurs the error and stack trace will helps us isolate the problem.

As for process.on('exit', () => this.shutdown());, this shouldn't have effects on swallowing up errors. Try attaching a process.on('uncaughtException', err => handler to see if there's an uncaughtException, or override process.exit() by defining your own function that logs an error stack (i.e. console.error(Exiting ${new Error().stac})) if you need to find out if something is calling process.exit() manually.

BannerBomb commented 6 years ago

The output of node -v shows v10.8.0

wzhouwzhou commented 6 years ago

Have you tried to run the test file directly outside of your bot with node without compiling?

BannerBomb commented 6 years ago

not yet. I will try that now.

BannerBomb commented 6 years ago

ok I tested a bit more it crashes if the description returns a "" string Because I tested the package with a different video and it worked just fine. Using the same steps as above I used the video Ozzy Osbourne - Crazy Train which returns a "" as the result.description which is when the crash occurs then I tested with the video Lamb of God - Blood of the Scribe which returned the videos description as well as not crashing my bot here is the 2 code blocks I used. I tested them with NPM Runkit to see what they returned as a result. (Sorry about all the edits on this one msg)

const { YTSearcher } = require('ytsearcher');
const search = new YTSearcher({
        key: YOUTUBE_API_TOKEN,
        revealkey: true
});
search.search("Lamb of God - Blood of the Scribe", { type: 'video' }).then(searchResult => {
    let result = searchResult.first;
    console.log(result.description); // Returns the videos description without crashing
}).catch(err => console.log(err.toString()));
const { YTSearcher } = require('ytsearcher');
const search = new YTSearcher({
        key: YOUTUBE_API_TOKEN,
        revealkey: true
});
search.search("Ozzy Osbourne - Crazy Train", { type: 'video' }).then(searchResult => {
    let result = searchResult.first;
    console.log(result.description); // using the video specified above returns "" and crashes.
}).catch(err => console.log(err.toString()));

and I checked the Ozzy Osbourne - Crazy Train video on youtube which does not have a description so the package is just calling on an undefined value. If you want to see here are the links to the videos. Ozzy Osbourne - Crazy Train: https://www.youtube.com/watch?v=ZDZtbBZuqb0 Lamb of God - Blood of the scribe: https://www.youtube.com/watch?v=6OArALRZAnE

The youtube links are the links of the videos it finds as the very first result from searching the video title that is why the link in my very first comment on this issue worked because the link leads to a video with a description. But searching from the videos title finds the specific video that does not have a description on the video.

wzhouwzhou commented 6 years ago

I just ran the second code bit on run kit and on my machines for v8, 9, and 10 and they didn't crash for me. The only conclusion I can see from this case is that some way you're using or handling the empty string description is causing a crash, so I believe that the problem does not lie within my package. Unfortunately I do not provide support in this issue tracker regarding one's specific usage and environment of the package if the package itself does not have any issues with regards to fetching and yielding data from google. Ordinarily, accessing a variable with an empty string should not throw any errors as I am not specifying getters/setters on the search result object. Any errors, at least to my knowledge, should report up the stack and unfortunately with the lack of error here it makes it very hard to pinpoint exactly what is happening. If somehow that previous versions worked and you believe that using those work you can specify those versions npm i -S ytsearcher@1.x.y, or with yarn add ytsearcher@1.x.y. A key difference between the latest version and earlier ones was that earlier ones used a package called needle for making web requests, and the latest ones use a smaller one called snekfetch. I plan on eventually transitioning this package to have no external dependencies, relying only on modules shipped with node. However, if certain search queries work for you and testing shows no errors then I don't believe the problem lies with which module is being used for web requests.

BannerBomb commented 6 years ago

ok, thanks for taking time to help though. And also I am sure you know this already but snekfetch has been deprecated and it recommends node-fetch instead just letting you know if you didn't see that yet. But again thanks. I might be able to do workaround for the issue.

wzhouwzhou commented 6 years ago

Yea, I'll release a minor version update when I get the time to work around the snekfetch deprecation. Going to be closing this issue now but I'll comment about it when I publish.

wzhouwzhou commented 6 years ago

Issue #2 Marked as Closed Wed Aug 15 ~11:30PM GMT-4 Do you enjoy using this package? If so, consider leaving a star on this repository, and checking out my other work, including my latest release easypathutil! Have an improvement you want to contribute? If so, please consider creating a pull request! Thank you for your time opening this issue, I hope you found my support useful, and if you have any other questions feel free to let me know.

wzhouwzhou commented 6 years ago

Good afternoon,

Checking in here again. I've released ytsearcher@1.2.0 on npm which removes 3rd party dependencies from the package; you can install it with npm i -S ytsearcher@1.2.0 or manually changing package.json and your lockfile so that yarn works as well (https://yarnpkg.com/en/package/ytsearcher). Give it a try and let me know if this update fixes things on your end.

~ William

BannerBomb commented 6 years ago

ok, sorry for the delay I just checked my notifications but I'll test it now.

BannerBomb commented 6 years ago

ok it works perfect now thanks for fixing the issue.