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

Download document pdfs is opening with error. #305

Open
abhim1509 opened this issue Aug 8, 2022 · 20 comments
Open

Download document pdfs is opening with error. #305

abhim1509 opened this issue Aug 8, 2022 · 20 comments

Comments

@abhim1509
Copy link

abhim1509 commented Aug 8, 2022

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
#255
https://developers.docusign.com/docs/esign-rest-api/how-to/download-envelope-documents/
MicrosoftTeams-image (10)
sample-documents (13).pdf

@abhim1509 abhim1509 changed the title ownload document pdfs. Download document pdfs is opening with error. Aug 8, 2022
@InbarGazit
Copy link
Member

what do you pass for documentId?

@abhim1509
Copy link
Author

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
Copy link
Author

@InbarGazit looking for your valuable inputs.

@andercs
Copy link

andercs commented Aug 26, 2022

@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
Copy link

andercs commented Aug 26, 2022

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
Copy link

andercs commented Aug 26, 2022

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
Copy link

andercs commented Aug 26, 2022

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.

andercs added a commit to andercs/docusign-esign-node-client that referenced this issue Aug 26, 2022
This commit corresponds to docusign#305 and provides a fix in line with the suggestions made therein.
andercs added a commit to andercs/docusign-esign-node-client that referenced this issue Aug 26, 2022
This commit corresponds to docusign#305 and provides a fix in line with the suggestions made therein.
@InbarGazit
Copy link
Member

@InbarGazit
Copy link
Member

  let mimetype;
  if (pdfFile) {
    mimetype = "application/pdf";
  } else if (docItem.type === "zip") {
    mimetype = "application/zip";
  } else {
    mimetype = "application/octet-stream";
  }

@andercs
Copy link

andercs commented Aug 26, 2022

@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
Copy link

andercs commented Aug 26, 2022

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
Copy link

zero245 commented Oct 14, 2022

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
Copy link

andercs commented Oct 17, 2022

@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
Copy link

ahmed-shorim-DS commented Oct 25, 2022

Have you tried adding the following 2 header values:

  • Accept: application/pdf
  • Content-Transfer-Encoding: base64

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
Copy link

andercs commented Oct 25, 2022

@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, #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
Copy link

@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
Copy link

@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
Copy link

andercs commented Oct 26, 2022

@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
Copy link

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

@alberto-maurel
Copy link

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.

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

6 participants