docusign / docusign-esign-node-client

The Official DocuSign Node.js Client Library used to interact with the eSign REST API. Send, sign, and approve documents using this client.
http://docusign.github.io/docusign-esign-node-client
MIT License
145 stars 99 forks source link

Download document pdfs is opening with error. #305

Open abhim1509 opened 1 year ago

abhim1509 commented 1 year ago

We are trying to download pdfs, I am sending binary data to frontend, however as soon as we open downloaded pdf, it pops up with error "We cant open this file. Something went wrong". Attaching the screenshots for reference. Kindly provide some suggestions on the _same. Have already looked in backend example citation and API documentation viz. https://github.com/docusign/code-examples-node/blob/master/lib/eSignature/examples/envelopeGetDoc.js https://github.com/docusign/docusign-esign-node-client/issues/255 https://developers.docusign.com/docs/esign-rest-api/how-to/download-envelope-documents/ MicrosoftTeams-image (10) sample-documents (13).pdf

InbarGazit commented 1 year ago

what do you pass for documentId?

abhim1509 commented 1 year ago

I have tried with the documentId, "combined" value. This is the current implementation with multiple documents and their documentIds: let documentsList = await envelopesApi.listDocuments( accountId, envelopeId, null ); const downloadedDocuments = await Promise.all( documentsList.envelopeDocuments.map(async (document: any) => { return await envelopesApi.getDocument( accountId, envelopeId, document.documentId, {} ); }) ); return downloadedDocuments; Here is the response in frontend.. image

abhim1509 commented 1 year ago

@InbarGazit looking for your valuable inputs.

andercs commented 1 year ago

@InbarGazit Would like to chime in that we're also getting this error, although we retrieve our documents slightly differently than Abhinav mentioned.

async getDocument(envelopeId: string, documentId?: string) {
        const envelopesApi = await this.getEnvelopesApi();
        const options = {};
        // options.certificate = 'false';
        return await envelopesApi.getDocument(
            this.accountId,
            envelopeId,
            documentId ? documentId : 'combined',
            options
        );
    }

Interestingly, the completed envelope can be readily downloaded from the eSignature Web App, so it's certainly stored as a valid PDF somewhere!

andercs commented 1 year ago

Quick follow-on, the example using curl at https://developers.docusign.com/docs/esign-rest-api/reference/envelopes/envelopedocuments/get/ works completely fine. I get a valid PDF when I save the output to a file, so the endpoint itself doesn't appear to be the issue.

Example:

curl --request GET 'https://demo.docusign.net/restapi/v2/accounts/0cdb3ff3-xxxx-xxxx-xxxx-e43af011006d/envelopes/ea4cc25b-xxxx-xxxx-xxxx-a67a0a2a4f6c/documents/1/' \
       --header 'Authorization: Bearer eyJ...bqg' --output test_file.pdf
andercs commented 1 year ago

Possible SOLUTION, it looks like the logic for this SDK to run callApi and handle the application/pdf response type was written on February 18, 2016.

This looks to predate y'all's superagent dependency's implementation of an appropriate application/pdf response type handler, which didn't occur until October 2, 2017.

My guess is that at least some, if not all of the odd behavior is originating from the usage of a string to store the byte chunks rather than an array like in the most modern official implementation from superagent.

https://github.com/visionmedia/superagent/blob/master/src/node/parsers/image.js

compared to the following from this library's ApiClient.js

if (request.header['Accept'] === 'application/pdf') {
      request.parse( function (res, fn) {
        res.data = '';
        res.setEncoding('binary');
        res.on( 'data', function (chunk) { res.data += chunk; } );
        res.on( 'end', function () {
          try {
            fn( null, res.data );
          } catch ( err ) {
            fn( err );
          }
        });
      })
    }

Since I'm guessing upgrading from v3.8.2 of SuperAgent to the most recent of v8.0.0 would be an endeavor, I'd yoink their function from image.js above and drop in a thank you comment in the code for it, presuming it fixes this issue.

andercs commented 1 year ago

Because I don't leave well enough alone, yeah my above suggestion fixes it. There's 35 bytes different between using curl and using the SDK with my suggested fix, but I can't visually distinguish it and both PDF files open.

InbarGazit commented 1 year ago

See code in https://github.com/docusign/code-examples-node/blob/master/lib/eSignature/examples/envelopeGetDoc.js which work ok for me.

InbarGazit commented 1 year ago
  let mimetype;
  if (pdfFile) {
    mimetype = "application/pdf";
  } else if (docItem.type === "zip") {
    mimetype = "application/zip";
  } else {
    mimetype = "application/octet-stream";
  }
andercs commented 1 year ago

@InbarGazit the only part of that example we care about is the fileBytes / results object that gets returned at the very end. We do the

let dsApiClient = new docusign.ApiClient();
  dsApiClient.setBasePath(args.basePath);
  dsApiClient.addDefaultHeader("Authorization", "Bearer " + args.accessToken);
  let envelopesApi = new docusign.EnvelopesApi(dsApiClient),
    results = null;

  // Step 1. EnvelopeDocuments::get.
  // Exceptions will be caught by the calling function
  results = await envelopesApi.getDocument(
    args.accountId,
    args.envelopeDocuments.envelopeId,
    args.documentId,
    null
  );

without issue. The problem is when you write those result bytes to a "PDF" file, no PDF reader can open them. The file is corrupt.

We don't use a mimeType for anything so the code from the comment a few minutes ago doesn't help us. I did most of the analysis for you all and even made an MR with the fix, which I tested locally and it does in fact fix the corrupt PDF issue.

andercs commented 1 year ago

Also opened a support Case in the DocuSign Support site to get more visibility into the issue because it's going to be a major blocker for us that I'd rather not have to come up with a workaround for.

Case #: 10013266

zero245 commented 1 year ago

Thanks andercs. We are also having the same exact problem. The response return by getDocument looks like PDF, but it won't open. Also recently, the API seems to act up when I set documentId to be certificate it doesn't download certificate, but a invalid PDF. When download portfolio then it downloads the certificate. This just happened right before I post this.

Docusign do you guys have a fix for this. Not being able to get the PDF properly via API is huge issue for using your service.

andercs commented 1 year ago

@zero245 yeah so they blew me off completely. The support case has been closed and they refused to leave it open until it was actually fixed. In addition, I was informed they only do twice yearly releases on the SDKs, so there's no intention of doing anything until at least the next one (whenever that happens to be).

With all that being said, they straight up told me to implement a workaround and query their API directly instead of using their SDK (which we did), but all-in-all it was a terrible experience and we plan to move to a competitor in the near future.

ahmed-shorim-DS commented 1 year ago

Have you tried adding the following 2 header values:

Adding the above header values should look similar to something like this:

const dsApi = new docusign.ApiClient()

dsApi.addDefaultHeader('Authorization', 'Bearer ' + accessToken)
dsApi.setBasePath('https://demo.docusign.net/restapi')
dsApi.addDefaultHeader('Accept', 'application/pdf')
dsApi.addDefaultHeader('Content-Transfer-Encoding', 'base64')

const envelopesApi = new docusign.EnvelopesApi(dsApi)
const envDoc = await envelopesApi.getDocument(accountId, envelopeId, "combined")

var buffer = Buffer.from(envDoc, 'base64')
fs.writeFile("test.pdf", buffer, function (err) {
    if (err) {
        console.log(err);
        return;
    }
})
andercs commented 1 year ago

@ahmed-shorim-DS looking at the code from this SDK (specifically EnvelopeApi.js) it would appear that application/pdf is included already when you make the getDocument call, so that should be already done by default. No reason to add it again unless there's a separate bug in this SDK's code.

For our workaround that calls the DocuSign API directly (bypassing the SDK) we don't have to specify base64 and it works fine. Same if you do a simple curl request from your terminal. No base64 encoding specification is necessary.

Doing the Content-Transfer-Encoding as base64 should either be done by default in the same EnvelopesApi method if there's some hidden requirement to use it for that method alone, or it shouldn't be necessary at all.

As I pointed out in my MR, https://github.com/docusign/docusign-esign-node-client/pull/306, the primary problem lies with the handler code for putting the application/pdf as a header on the request.

My MR can't work here apparently because you all support as far back as Node v4, which doesn't have Buffer support, but Buffer is a wrapper around yet other code that you all could pull from and implement to actually fix the underlying PDF parsing issue.

ahmed-shorim-DS commented 1 year ago

@andercs Yes, you can ignore the application/pdf part but if you add the base64 part you should have a valid PDF document downloaded. Kindly try it and tell me if it works for you.

Regarding having it by default in the EnvelopesApi, I am in touch with the engineering team for that specific topic.

Thank you.

ahmed-shorim-DS commented 1 year ago

@andercs Another way to do it without setting the base64 header would be as follow:

const env = await envelopesApi.getDocument(accountId, envelopeId, "combined");

let writeStream = fs.createWriteStream('test.pdf');
writeStream.write(env, 'binary');
writeStream.on('finish', () => {
    console.log('file saved');
});
writeStream.end();

Since the endpoint gives you back the file in binary format not in base64

andercs commented 1 year ago

@ahmed-shorim-DS at this point, your suggestions are appreciated for the benefit of others, but as we did not receive this more helpful level of support for about two months, we implemented the workaround (as suggested by the technical support staff), deleted any reference to envelopesApi.getDocument from our code (although we did keep both paths for the first month), and have had to move on to more pressing issues.

ahmed-shorim-DS commented 1 year ago

@abhim1509 @zero245 Does the above code snippet that I provided help you?

alberto-maurel commented 10 months ago

I'm having the same issue. Using Content-Transfer-Encoding: base64 fixes it:

// Setting the header
this.docusignApiClient.addDefaultHeader(
  'Content-Transfer-Encoding',
  'base64'
);

// Performing the request
const results = await envelopesAPI.getDocument(
  account_id,
  envelopeId,
  'combined'
);
const buff = Buffer.from(results, 'base64');
await fs.writeFileSync('test_7.pdf', buff);

Thanks @andercs for flagging it, definitely saved me, and @ahmed-shorim-DS for the workaround.