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

Application fails when package.json is absent #508

Closed
4 tasks done
Azarattum opened this issue Sep 16, 2023 · 6 comments · Fixed by #509
Closed
4 tasks done

Application fails when package.json is absent #508

Azarattum opened this issue Sep 16, 2023 · 6 comments · Fixed by #509
Labels
bug Something isn't working

Comments

@Azarattum
Copy link
Contributor

Steps to reproduce

My node application is bundled for deployment, therefore there’s no package.json file in the final build.

const package_json = JSON.parse(readFileSync(path.resolve(__dirname__, is_cjs ? '../package.json' : '../../package.json'), 'utf-8'));

This assumes that the file is always present and crashes my app.

Failure Logs

ENOENT: no such file or directory, open '/home/package.json'
    at Object.openSync (node:fs:592:3)
    at Object.readFileSync (node:fs:460:35)
    at Object.<anonymous> (/home/beta_amadeus/plugins/youtube.cjs:82:39138)
    at Module._compile (node:internal/modules/cjs/loader:1255:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1309:10)
    at Module.load (node:internal/modules/cjs/loader:1113:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:165:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:

Expected behavior

The library should continue working without package.json, as it doesn’t contain any essential information for it to function. In fact, putting an empty (with just {}) package.json in my home folder fixes the issue. But this is a bad solution. My app isn’t even in the home directory…

Current behavior

The library crashes.

Version

Default

Anything else?

No response

Checklist

  • I am running the latest version.
  • I checked the documentation and found no answer.
  • I have searched the existing issues and made sure this is not a duplicate.
  • I have provided sufficient information.
@Azarattum Azarattum added the bug Something isn't working label Sep 16, 2023
@absidue
Copy link
Collaborator

absidue commented Sep 16, 2023

Unfortunately unlike with the deno and web builds, there is no way for the node one to import the json, as it needs to be able to support both commonjs and es modules, but importing json in esmodules has been experimental in node for a long time (and I'm pretty sure it still is).

So the choices are:

  1. the current solution, which works for everyone that doesn't try to bundle the node version of YouTube.js
  2. require everyone to pass a node experiment flag every time they use YouTube.js on node

@Azarattum
Copy link
Contributor Author

Can't we just default to {} if file doesn't exist instead of crashing?

@absidue
Copy link
Collaborator

absidue commented Sep 16, 2023

Well if you never plan on reporting any errors to YouTube.js sure, but as the values in the packages.json file are used to generate the error logs, having them is kind of important.

And no there is no way to specify those values at build time because the typescript compiler doesn't support that and if we did a find and replace after the build, the source maps would be incorrect.

@Azarattum
Copy link
Contributor Author

Well if you never plan on reporting any errors to YouTube.js sure

This is better than it crashing, though. Also, you always can check the version manually before reporting anything.

@absidue
Copy link
Collaborator

absidue commented Sep 16, 2023

You are expecting too much from people, lots of the issues on this repo are just people copy pasting the error messages and warnings, they don't provide any context whatsoever. Not even the video or channels they were accessing when the issue occurred.

I guess that fallback could be added, probably worth printing an error when it does fall back, saying that the node version of YouTube.js is incompatible with bundling.

@Azarattum
Copy link
Contributor Author

I think I've found a decent solution. @absidue, take a look at #509.

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

Successfully merging a pull request may close this issue.

2 participants