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

Errors propogating from callbacks defined in fileWriteStreamHandler fall outside of the scope of formidable #826

Closed
cubesnyc opened this issue Feb 8, 2022 · 1 comment

Comments

@cubesnyc
Copy link

cubesnyc commented Feb 8, 2022

Support plan

  • Which support plan is this issue covered by? (Community, Sponsor, Enterprise): Community
  • Currently blocking your project/work? (yes/no): yes
  • Affecting a production system? (yes/no): yes

Context

  • Node.js version: 16.13.2
  • Release Line of Formidable (Legacy, Current, Next):
  • Formidable exact version: 2.0.1
  • Environment (node, browser, native, OS): node
  • Used with (popular names of modules): aws-sdk, next, PassThrough

What are you trying to achieve or the steps to reproduce?

I am passing a fileWriteStreamHandler function to handle uploads to s3. This sends a passthrough stream to s3, and sets up a callback that runs after the upload has finished. This callback contains any errors that occured during the upload process. Throwing any errors that are defined in this callback falls outside of the scope of form.parse and the 'error' event. In fact, the 'end' event fires before this callback even runs. How are these errors supposed to be handled within the same scope as the form processing/parsing?

const uploadStreamHandler = (file) => {
	const passThrough = new PassThrough();
	const params = {
		Bucket: process.env.AWS_BUCKET,
		Key: file.newFilename,
		Body: passThrough,
	};

	const callback = (err, data) => {
		if (err) {
			throw new Error(err + JSON.stringify(data));
		}
		console.log("done s3"); <-- this runs after form.parse is finished
		throw new Error("s3callback"); <---- this does not trigger an error event or err in form.parse callback
	};

	s3Client.upload(params, callback);
	return passThrough;
};

What was the result you got? As stated above, throwing an error from within the callback defined in the fileWriteStreamHandler is outside the scope of formidable. This presents an error handling challenge.

What result did you expect? I would expect all errors related form parsing/processing to have a common scope within which they can be handled. This currently does not seem to be the case

@cubesnyc cubesnyc added the bug label Feb 8, 2022
@GrosSacASac
Copy link
Contributor

First of all the formidable library cannot control when s3Client.upload is going to call the callback function.

There are 2 ways to throw an error while it parsing that will be caught in the form.parse error,

  1. use new Error inside next inside the write callback of the stream
  2. or emit directly an error with form.emit('error', 'my error')
import http from 'node:http';
import { Writable } from 'node:stream';
import formidable from '../src/index.js';

const way1 = (/* file */) => {
    const writable = Writable();
    // eslint-disable-next-line no-underscore-dangle
    writable._write = (chunk, enc, next) => {
      console.log(chunk.toString());
      next(new Error("error in custom stream"));
    };
    return writable;
};

const way2 = (/* file */) => {
  const writable = Writable();
  writable._write = (chunk, enc, next) => {
    form.emit('error', 'error in custom stream');
    next();
  };
  return writable;
};

const server = http.createServer((req, res) => {
  if (req.url === '/api/upload' && req.method.toLowerCase() === 'post') {
    // parse a file upload
    const form = formidable({
      // fileWriteStreamHandler: ,
      fileWriteStreamHandler: way1 || way2,
    });

    form.parse(req, (err, fields, files) => {
      if (err) {
        console.error('error inside form.parse');
        console.error(err);
        res.writeHead(err.httpCode || 400, { 'Content-Type': 'text/plain' });
        res.end(String(err));
        return;
      }
      res.writeHead(200, { 'Content-Type': 'application/json' });
      res.end(JSON.stringify({ fields, files }, null, 2));
    });

    return;
  }

  // show a file upload form
  res.writeHead(200, { 'Content-Type': 'text/html' });
  res.end(`
    <h2>With Node.js <code>"http"</code> module</h2>
    <form action="/api/upload" enctype="multipart/form-data" method="post">
      <div>Text field title: <input type="text" name="title" /></div>
      <div>File: <input type="file" name="file" /></div>
      <input type="submit" value="Upload" />
    </form>
  `);
});

server.listen(3000, () => {
  console.log('Server listening on http://localhost:3000 ...');
});

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

No branches or pull requests

2 participants