Skip to content

LargeFileUploadTask is ending with PassThrough stream instead of saying JSON response with id #359

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

Closed
1 task
hilarudeens opened this issue Nov 19, 2020 · 11 comments · Fixed by #397
Closed
1 task
Assignees
Labels
ADO to GitHub automation label Issue caused by core project dependency modules or library Bug - P1 bug: client
Milestone

Comments

@hilarudeens
Copy link

hilarudeens commented Nov 19, 2020

Bug Report

I'm using msgraph-sdk-javascript in NodeJs and try attach a document in Outlook email. The issue is, LargeFileUploadTask.uploadSlice function returns PassThrough readable stream on end of file upload instead of JSON response with id.

Prerequisites

  • [ Yes] Can you reproduce the problem?
  • [Yes ] Are you running the latest version?
    I'm using 2.1.1 as of 19th Nov 2020
  • [Yes ] Are you reporting to the correct repository?
  • Did you perform a cursory search?

Steps to Reproduce

  1. Create an Outlook draft message by usingPostman.

  2. Create a simple HttpTrigger Azure Function and Add following code in Azure Function handler.

const saveAttachment = async (oid, tenantId, messageId, file) => {
    // getClient is just creating client with GraphToken generate authProvider.
    // authProvider is doing client-credential flow to generate token.
    const client = await getClient(tenantId);

    const uploadSessionPayload = {
        AttachmentItem: {
          attachmentType: 'file',
          name: file.filename,
          size: file.size
        }
    };

    const uploadSession = await LargeFileUploadTask.createUploadSession(
        client, 
        `/users/${oid}/messages/${messageId}/attachments/createUploadSession`,
        uploadSessionPayload,
    );
    const fileObject = {
        content: file.data,
        name: file.filename,
        size: file.size
    };

    const { url: uploadUrl } = uploadSession; 
    const uploadUrlTokens = uploadUrl.split('?');
    const uploadUrlAuthToken = uploadUrlTokens[1].replace('authtoken=', '');
    const uploadClient = Client.init({
        authProvider: (done) => done(null, uploadUrlAuthToken),
        debugLogging: true
    });

    const uploadTask = new LargeFileUploadTask(uploadClient, fileObject, uploadSession, {
        rangeSize: 5 * 1024 * 1024,
    });

    const uploadResponse = await uploadTask.upload();
    return uploadResponse;
};
  1. Execute AzureFunction in Visual Studio Code.

  2. Call HttpTrigger from Postman

Expected behavior:
Large file should be uploaded in sequential PUT call. In end of all chunks upload, LargeFileUploadTask.uploadSlice function should give response with 'id'

ShouldEndUpFileJsonResponseWithIdProperty

Actual behavior:
LargeFileUploadTask.uploadSlice function is giving NodeJs ReadableStream.

Additional Context

LargeFileUploadResponse1

LargeFileUploadResponseEndOfUpload

Code Line:

AB#7549

@ghost ghost added the ToTriage label Nov 19, 2020
@nikithauc
Copy link
Contributor

@hilarudeens Thank you for reporting this issue!

I will take a look at this issue.

@nikithauc
Copy link
Contributor

@hilarudeens
The LargeFileUploadTask is designed for uploading to OneDrive. I see that you are trying to upload an attachment to Outlook. The responses by Drive and Outlook differ from each another. Please refer to this https://docs.microsoft.com/en-us/graph/outlook-large-attachments?tabs=javascript. In the document you will see that the final response from outlook is expected to be an empty body with the Location (of the attachment you uploaded) header.
Please let me know if that helps.

Thank you @ificator for your help with this issue!

@nikithauc nikithauc reopened this Nov 30, 2020
@nikithauc nikithauc modified the milestone: 2.2.0 Nov 30, 2020
@nikithauc nikithauc self-assigned this Dec 4, 2020
@nikithauc
Copy link
Contributor

TODO - The current LargeFileUpload task implementation in JS sdk needs an update. We should be allowing uploads to Outlook and the design should not be tightly coupled with the response format of one service like the current implementation .

@hilarudeens
Copy link
Author

hilarudeens commented Dec 4, 2020

@nikithauc Thanks for responding.

I'm expecting a way to get id. I'm okay to read Location header, however the current SDK implementation is giving passThrough stream. I'm unable to read Location header from passThrough.

Do I need to wait for SDK upgrade? or is there any workaround?

@nikithauc
Copy link
Contributor

@hilarudeens There will be an design change for this issue.

A suggested workaround would be following this https://docs.microsoft.com/en-us/graph/outlook-large-attachments?tabs=javascript documention.
On creation of the upload session as shown in the documentation, you can use a pattern similar to upload and change the condition to end the upload when the location or success condition is met.

I apologize for the inconvenience. Please let me know if you have any questions.

@nikithauc
Copy link
Contributor

@clawyq
Copy link

clawyq commented Feb 4, 2021

this issue of the sdk returning a passThrough stream can also be reproduced by making a call to the copy endpoint as specified here (specified in the 'initial action request' section).

@nikithauc
Copy link
Contributor

@clawyq Thanks for bringing this to our attention! This issue will be addressed in 3.0.0.

@nikithauc
Copy link
Contributor

@clawyq Can you please check if the following works for you -

let res = await client.api('/me/drive/items/{folder-item-id}/copy').responseType(ResponseType.RAW).
	.version('beta')
	.post(driveItem);

using responseType(ResponseType.RAW) should fetch the entire response with the headers.

@clawyq
Copy link

clawyq commented Feb 7, 2021

@nikithauc I was able to retrieve the monitor URL through the steps detailed here. It would be very helpful if there was documentation for this flow with the API.

Also, does the graph client support querying for the progress of the operation? Currently, after obtaining the monitor URL, I will then use axios to get the status of the operation - was wondering if the SDK itself natively provides the ability to query for the status of the operation. Thanks!

@nikithauc
Copy link
Contributor

@clawyq can you try the following ?

let res = await client.api(<monitor url>)
	.version(<v1.0/beta>)
	.get();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADO to GitHub automation label Issue caused by core project dependency modules or library Bug - P1 bug: client
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants