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

getDatasetSiteTemplate callback error #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

drinkynet
Copy link

Fix for: [ERROR] TypeError [ERR_INVALID_CALLBACK]: Callback must be a function. Received { encoding: 'utf8' }. Use fs.promises.readFile() which doesn't need a callback argument

Fix for: [ERROR] TypeError [ERR_INVALID_CALLBACK]: Callback must be a function. Received { encoding: 'utf8' }. Use fs.promises.readFile() which doesn't need a callback argument
@nickevansuk nickevansuk mentioned this pull request Oct 5, 2023
@nickevansuk
Copy link
Contributor

DO NOT MERGE: Note that by default any PR on this repo may release a new version and cause a breaking change, so we need to work around this during release.

@lukehesluke
Copy link

lukehesluke commented Oct 9, 2023

@nickevansuk the getDatasetSiteTemplate() function does not work presently. This PR fixes that. So I don't think it's a breaking change so much as it is a necessary fix.

Here's what happens if I run the function presently (using the same node version as in package.json, v14.16.0):

// file: test.js
const { getDatasetSiteTemplate } = require('./lib');

main();

async function main() {
  const x = await getDatasetSiteTemplate(true);
  console.log(x);
}
# terminal
$ node test.js
(node:17983) UnhandledPromiseRejectionWarning: TypeError [ERR_INVALID_CALLBACK]: Callback must be a function. Received { encoding: 'utf8' }
    at maybeCallback (fs.js:160:9)
    at Object.readFile (fs.js:311:14)
    at getDatasetSiteTemplate (/Users/lukewinship/Dev/openactive/dataset-site-template/lib/index.js:19:19)
    at main (/Users/lukewinship/Dev/openactive/dataset-site-template/test.js:6:19)
    at Object.<anonymous> (/Users/lukewinship/Dev/openactive/dataset-site-template/test.js:3:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:17983) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:17983) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Then, if I run it with the fix from this patch applied, I get:

# terminal
$ node test.js
<!DOCTYPE HTML>
<!--
  OpenActive Dataset Site Template version 7, from https://unpkg.com/@openactive/dataset-site-template@7.0.0/dist/datasetsite.mustache
-->
# ... etc

@nickevansuk
Copy link
Contributor

nickevansuk commented Oct 9, 2023

Sorry I wasn't clear in my previous comment @lukehesluke, agreed this fixes the issue, however the problem is that merging this will currently trigger CI which will cause a major version bump (which as you say is unnecessary, and is also disruptive to current implementations)

The issue is this line:

# TODO: Make this conditional on if template files have actually changed
run: npm version major

Which must be resolved before this can be merged. Hopefully something that we can sort early in the next round of KT.

@lukehesluke
Copy link

For context, this is the (presently draft) issue that fixes the versioning issue: https://github.com/orgs/openactive/projects/17/views/1?pane=issue&itemId=35347051

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Backlog
Development

Successfully merging this pull request may close these issues.

None yet

4 participants