-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
node.fetch ESM import issue in getFileByURL.js #4421
Comments
According to node-fetch the following should be used on CJS: // mod.cjs
const fetch = (...args) => import('node-fetch').then(({default: fetch}) => fetch(...args)); https://www.npmjs.com/package/node-fetch The CJS and ESM syntax is different and TypeScript code gets here transpiled to CJS syntax which is wrong. General question: why not using Node.js' fetch on 20.x? |
Fails here:
|
Patched the transpiled getFileByURL.js file: "use strict";
Object.defineProperty(exports, "__esModule", {
value: true
});
Object.defineProperty(exports, "default", {
enumerable: true,
get: function() {
return _default;
}
});
//const _nodefetch = /*#__PURE__*/ _interop_require_default(require("node-fetch"));
const _path = /*#__PURE__*/ _interop_require_default(require("path"));
function _interop_require_default(obj) {
return obj && obj.__esModule ? obj : {
default: obj
};
}
const getFileByURL = async (url)=>{
if (typeof url === 'string') {
const res = await fetch(url, {
headers: {
'Content-Type': 'application/json'
},
method: 'GET'
});
const data = await res.buffer();
const name = _path.default.basename(url);
return {
name,
data,
mimetype: res.headers.get('content-type') || undefined,
size: Number(res.headers.get('content-length')) || 0
};
}
};
const _default = getFileByURL; |
See discussion here: node-fetch/node-fetch#1279 (comment) |
That file was recently introduced by #4170 |
Having updated to Payload 2.4.0, I'm currently unable to I'm currently on Node 20.8.0.
|
Hi @cbratschi @DavidOliver this has been fixed and will be out in our next release. Thanks for reporting! |
Hey, I wanna start a new project but I can't because it hasn't been released yet. Is there a way I can start using payload before the relase ? |
I'd also appreciate a new release. :) Thanks for fixing. @AhmetHuseyinDOK , you could specify v2.3.1 in your package.json file and |
I checked the latest Payload version and there is still an issue left: the getExternalFile() call is silently failing:
I modified the code to get the thrown exception:
https://github.com/payloadcms/payload/blob/main/packages/payload/src/uploads/getExternalFile.ts#L15 The transpiled TypeScript code is: const { default: fetch } = await Promise.resolve().then(()=>/*#__PURE__*/ _interop_require_wildcard(require("node-fetch"))); This is still a require() call and not a dynamic import. Could you please pass all caught exceptions as cause to a secondary exception: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause. In that case no exceptions get lost and it's mich easier to debug such issues. |
Could you please re-open this issue? This is still reproducible. |
Could you please re-open and patch according to my description below. Error HandlingFirst log the error on console like on this line in the same file:
Then modify it here:
Here is a patch for the transpiled code: try {
if (url && url.startsWith('/') && !disableLocalStorage) {
const filePath = `${staticPath}/${filename}`;
const response = await (0, _getFileByPath.default)(filePath);
file = response;
overwriteExistingFiles = true;
} else if (filename && url) {
file = await (0, _getExternalFile.getExternalFile)({
req,
data: data
});
overwriteExistingFiles = true;
}
} catch (err) {
console.error(err);
throw new _errors.FileUploadError(req.t);
} Now we see all errors. Better would be to pass the error to FileUploadError:
class FileUploadError extends APIError {
constructor(t?: TFunction, cause: Error) {
super(
t ? t('error:problemUploadingFile') : 'There was a problem while uploading the file.',
httpStatus.BAD_REQUEST,
)
this.cause = cause
}
} node-fetch FixNow we see the root cause of the error:
Problem occurs in transpiled code here:
This is the patch for the transpiled code (replace node-fetch by fetch): const getExternalFile = async ({ data, req })=>{
const { filename, url } = data;
if (typeof url === 'string') {
let fileURL = url;
if (!url.startsWith('http')) {
const baseUrl = req.get('origin') || `${req.protocol}://${req.get('host')}`;
fileURL = `${baseUrl}${url}`;
}
//Note: replaced node-fetch by fetch
//const { default: fetch } = await Promise.resolve().then(()=>/*#__PURE__*/ _interop_require_wildcard(require("node-fetch")));
const res = await fetch(fileURL, {
credentials: 'include',
headers: {
...req.headers
},
method: 'GET'
});
if (!res.ok) throw new _errors.APIError(`Failed to fetch file from ${fileURL}`, res.status);
//const data = await res.buffer();
const data = Buffer.from(await res.arrayBuffer());
return {
name: filename,
data,
mimetype: res.headers.get('content-type') || undefined,
size: Number(res.headers.get('content-length')) || 0
};
}
throw new _errors.APIError('Invalid file url', 400);
}; Basically these two lines are modified: //const { default: fetch } = await Promise.resolve().then(()=>/*#__PURE__*/ _interop_require_wildcard(require("node-fetch")));
const data = Buffer.from(await res.arrayBuffer()); |
@cbratschi Some fixes were pushed in this area. I'd be curious if you still see this issue in 2.14.2. Regarding replacing node-fetch with fetch. Unfortunately, this is not possible for us to do because Payload v2 supports node versions before 18 which was when the built-in fetch api was introduced. |
@denolfe Still the same. The only change is that the error is now better handled and its message is shown in a toast. Here is the full error message:
The node-fetch ESM module is still being loaded by require(). This would only work with Node's 22 experimental loader. You could downgrade node-fetch to v2. The TypeScript transpiler converts the async import() to a require() which is not easy to solve. |
I am trying to recreate this on |
@JarrodMFlesch the error happens for instance after saving resized images using non-local image storage. In that case Payload uses node-fetch to retrieve the original image data. This error does not occur while generating Payload types. A easy test case would be to directly import getFileByURL() and pass any URL. In previous Payload versions the exception was quietly ignored. So double check you get all exceptions. |
I ran into this exact error. I am saving my images to the AWS EFS file system, and the initial upload works fine but attempting to save a cropped image throws that node-fetch error. [17:00:43] ERROR (payload): FileRetrievalError: There was a problem while uploading the file. require() of ES Module /home/node/app/node_modules/node-fetch/src/index.js from /home/node/app/node_modules/payload/dist/uploads/getExternalFile.js not supported.
Instead change the require of index.js in /home/node/app/node_modules/payload/dist/uploads/getExternalFile.js to a dynamic import() which is available in all CommonJS modules.
at generateFileData (/home/node/app/node_modules/payload/dist/uploads/generateFileData.js:69:23)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async updateByID (/home/node/app/node_modules/payload/dist/collections/operations/updateByID.js:108:61)
at async updateByIDHandler (/home/node/app/node_modules/payload/dist/collections/requestHandlers/updateByID.js:41:21) I am on payload 2.18.3, on Node.js 20.10.0. |
We're facing exact same issue using Google Cloud Storage and in local development where we don't have external storage connected. Crop works fine when uploading an image but after uploading it, it throws the error mentioned above We're on Payload 2.23.1 and Node.js 20.11.0 |
Any solution to this yet? I am just starting out with the website template and I am getting this same error when I try to npm run dev in local development |
just realized this error occurs when updating the image focal point, in addition to resizing 🙁 has anybody managed to find a workaround other than OP's example of directly modding the transpiled code? |
Link to reproduction
private repo
Describe the Bug
Getting the following error in Payload 2.4.0 while trying to build the types:
To Reproduce
npm run payload:types is failing after upgrading to 2.4.0. All packages are the latest versions.
Payload Version
2.4.0
Adapters and Plugins
db-mongodb, bundler-webpack, live-preview
The text was updated successfully, but these errors were encountered: