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

doc.end() callback? #265

Closed
chrisspiegl opened this issue Aug 9, 2014 · 25 comments
Closed

doc.end() callback? #265

chrisspiegl opened this issue Aug 9, 2014 · 25 comments

Comments

@chrisspiegl
Copy link

Is there a way to get a callback after everything asynchronous is finished and the file is completely stored to the file system?

I can't find a way. Probably I am missing something?

@chrisspiegl
Copy link
Author

My workaround (I PDFkit should provide a callback method or promise):

Store the writeStream in a extra variable and watch it for a change:

doc = new PDFDocument();
writeStream = fs.createWriteStream('filename.pdf');
doc.pipe(writeStream);
doc.end()
writeStream.on('finish', function () {
    // do stuff with the PDF file
});

@devongovett
Copy link
Member

Yeah the finish event is how I would have done it. I suppose we could add a callback as a convenience. Do you think it's worth it given that the finish event is already there?

@chrisspiegl
Copy link
Author

I see two ways of doing it:

  1. Document the finish event somewhere near doc.end() to show people the easy way to wait for the finished document (so they do not have to search as long as I did :)).
  2. A finish callback - which I would prefer - because than I would not have to deal with the stream myself. Save it in a extra variable which I would otherwise not need and then listen to the event myself.

@obenjiro
Copy link

+1 for "finish callback".. this will be much more "intuitive"

@alexnaspo
Copy link

+1 for callback

@devongovett
Copy link
Member

The problem with adding a callback to the end method is that PDFKit doesn't actually know when all of the data has been flushed to whatever stream you're writing to (file, http response, etc.). Since PDFKit has no access to the actual writable stream it is being piped to (PDFKit itself is a readable stream, and you set up the writable part), it only knows when it has finished pumping out chunks to whoever might be reading. It may be some time later that the writable stream actually flushes its internal buffers out to the actual destination. So, we could add a callback to the end method, but it might get called before the stream is actually done writing. This could cause subtle and hard to debug race conditions in people's code who assume that everything is really done when that callback is called.

For this reason, my vote is to keep PDFKit the way it is now and not to add a callback to doc.end. It's not too hard to add a finish event handler to the stream you set up to pipe to if you need that, and it will prevent future headaches.

@chrisspiegl
Copy link
Author

I think it would be a great idea to document this then.
I had to search a lot to find this. Probably somewhere near the doc.end documentation.
On August 21, 2014 at 22:06:13, Alex Naspo (notifications@github.com) wrote:

+1 for callback


Reply to this email directly or view it on GitHub.

@danielflippance
Copy link

+1 for callback

@weeger
Copy link

weeger commented Dec 21, 2015

+1

@gggauravgandhi
Copy link

+1

Thanks a lot. I was facing weird error in my execution.
When executing multiple queries it was working proper and when running single query then pdf was not created properly.
Then realised this callback issue was there.
Thanks a lot for finish callback.

@codeandcats
Copy link

I believe it's not quite as simple as listening for the 'finish' event on the write stream, specifically in the case of errors, so I implemented the following function which returns a Promise.

function savePdfToFile(pdf : PDFKit.PDFDocument, fileName : string) : Promise<void> {
    return new Promise<void>((resolve, reject) => {

        // To determine when the PDF has finished being written successfully 
        // we need to confirm the following 2 conditions:
        //
        //   1. The write stream has been closed
        //   2. PDFDocument.end() was called syncronously without an error being thrown

        let pendingStepCount = 2;

        const stepFinished = () => {
            if (--pendingStepCount == 0) {
                resolve();
            }
        };

        const writeStream = fs.createWriteStream(fileName);
        writeStream.on('close', stepFinished);
        pdf.pipe(writeStream);

        pdf.end();

        stepFinished();
    }); 
}

Instead of calling .end() directly, you call this function and pass the pdf document and filename.

This should correctly handle the following situations:

  • PDF Generated successfully
  • Error is thrown inside pdf.end() before write stream is closed
  • Error is thrown inside pdf.end() after write stream has been closed

@askdesigners
Copy link

That's a nice solution. :) It would be great to see that in the library itself so that users don't have to deal with these issues.

@ctnitchie
Copy link

+1 for at least a note in the doc.

@MedinaGitHub
Copy link

+1 it is elementary

@SeanCannon
Copy link

I had a similar issue where I need to save the file to s3.

writeStream.on('finish', () => { /* s3 upload code here */ })

Worked great, thanks @spieglio.

@eyaylagul
Copy link

I had a similar issue where I need to save the file to s3.

writeStream.on('finish', () => { /* s3 upload code here */ })

Worked great, thanks @spieglio.

@SeanCannon
can you share your s3 pdf create code?

Because sometimes i can creating pdfFile successfully with AWS lambda. But sometimes pdf file created right(pdf file size is right) while but has a error so cant open file, or sometimes pdf file created wrong(file size is look 16).

I cant handle this situation.

@JulyTheMonth
Copy link

If anyone has trouble catching the error when a file has failed to upload, following code can be added to the function of codeandcats

writeStream.on("error", (e: Error) => {
    reject(e);
});

@mythicalbox
Copy link

mythicalbox commented Jan 1, 2021

This solved it for me:

    const stream = fs.createWriteStream(localFilePath);
    doc.pipe(stream);

    .....

    doc.end();

    await new Promise<void>(resolve => {
         stream.on("finish", function() {
              resolve();
         });
    });

It's a bit less verbose.. just waits before continuing.

@axmccx
Copy link

axmccx commented May 7, 2021

This solved it for me:

    const stream = fs.createWriteStream(localFilePath);
    doc.pipe(stream);

    .....

    doc.end();

    await new Promise<void>(resolve => {
         stream.on("finish", function() {
              resolve();
         });
    });

It's a bit less verbose.. just waits before continuing.

This fixed my issue, thanks for sharing!!

@ibadeeCodes
Copy link

My workaround (I PDFkit should provide a callback method or promise):

Store the writeStream in a extra variable and watch it for a change:

doc = new PDFDocument();
writeStream = fs.createWriteStream('filename.pdf');
doc.pipe(writeStream);
doc.end()
writeStream.on('finish', function () {
    // do stuff with the PDF file
});

I am creating many pdfs in a loop and in the end I am downloading all the resultant pdf's in a zip folder.
How could I identify if all the write streams are completed?

@axmccx
Copy link

axmccx commented Jul 11, 2021

For each PDF you're making, would you be able to create promises as shown in this thread, push them to an array, then feed that array to Promise.all and do an await on it?

@marchingband
Copy link

I am piping to an http response object inside a Firebase function.
I need to call res.end() when it's done. Without a callback from the library I'm not sure how to accomplish it, the response object has no idea when the library is done piping ... does it?
Currently I just setTimeout for a while to give it time ... not a real solution.

@blikblum
Copy link
Member

I have plan to make it fully sinchronously

@gndm-zrobank
Copy link

I believe it's not quite as simple as listening for the 'finish' event on the write stream, specifically in the case of errors, so I implemented the following function which returns a Promise.

function savePdfToFile(pdf : PDFKit.PDFDocument, fileName : string) : Promise<void> {
    return new Promise<void>((resolve, reject) => {

        // To determine when the PDF has finished being written successfully 
        // we need to confirm the following 2 conditions:
        //
        //   1. The write stream has been closed
        //   2. PDFDocument.end() was called syncronously without an error being thrown

        let pendingStepCount = 2;

        const stepFinished = () => {
            if (--pendingStepCount == 0) {
                resolve();
            }
        };

        const writeStream = fs.createWriteStream(fileName);
        writeStream.on('close', stepFinished);
        pdf.pipe(writeStream);

        pdf.end();

        stepFinished();
    }); 
}

Instead of calling .end() directly, you call this function and pass the pdf document and filename.

This should correctly handle the following situations:

  • PDF Generated successfully
  • Error is thrown inside pdf.end() before write stream is closed
  • Error is thrown inside pdf.end() after write stream has been closed

Saved my life hahah thanks!

@HelenLangers
Copy link

I believe it's not quite as simple as listening for the 'finish' event on the write stream, specifically in the case of errors, so I implemented the following function which returns a Promise.

function savePdfToFile(pdf : PDFKit.PDFDocument, fileName : string) : Promise<void> {
    return new Promise<void>((resolve, reject) => {

        // To determine when the PDF has finished being written successfully 
        // we need to confirm the following 2 conditions:
        //
        //   1. The write stream has been closed
        //   2. PDFDocument.end() was called syncronously without an error being thrown

        let pendingStepCount = 2;

        const stepFinished = () => {
            if (--pendingStepCount == 0) {
                resolve();
            }
        };

        const writeStream = fs.createWriteStream(fileName);
        writeStream.on('close', stepFinished);
        pdf.pipe(writeStream);

        pdf.end();

        stepFinished();
    }); 
}

Instead of calling .end() directly, you call this function and pass the pdf document and filename.

This should correctly handle the following situations:

  • PDF Generated successfully
  • Error is thrown inside pdf.end() before write stream is closed
  • Error is thrown inside pdf.end() after write stream has been closed

You are a genius and have saved my sanity. Thank you.

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