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

@uppy/tus: add response to upload-success #5444

Closed
2 tasks done
ikopmans opened this issue Sep 2, 2024 · 6 comments · Fixed by #5456
Closed
2 tasks done

@uppy/tus: add response to upload-success #5444

ikopmans opened this issue Sep 2, 2024 · 6 comments · Fixed by #5456
Assignees
Labels

Comments

@ikopmans
Copy link

ikopmans commented Sep 2, 2024

Initial checklist

  • I understand this is a bug report and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

No response

Steps to reproduce

uploader = new Uppy({ autoProceed: true, allowMultipleUploadBatches: true })
.use(Tus, { endpoint: 'http://localhost/upload' });
uploader.on('upload-success', (file, response) => console.log('Upload completed with response ', response));

Expected behavior

body field in response object contains response body, as returned from server (if any)

Actual behavior

response.body is always empty (hard-coded to be {}), even though it seems to be supported by underlying tus-js-client library

@ikopmans ikopmans added the Bug label Sep 2, 2024
@Murderlon
Copy link
Member

What would you expect to get for the body? And why do you need it?

@ikopmans
Copy link
Author

ikopmans commented Sep 3, 2024

tusd (which seems to be a protocol reference implementation) allows PreFinishResponseCallback for post-upload validation. In my case it checks if uploaded image is already uploaded by some another user using signature database. I can imagine many other reasons to validate file content after it was uploaded, like check for nudity and other explicit content and reject the file based on that. In all such cases there should be a way to respond that a file is rejected by server side. I guess that's the reason to have this callback. But so far I see no ways to accept this response on Uppy side.

tus-client-js has onSuccess event handler, that can accept such data:

onSuccess: function (payload) {
	  const { lastResponse } = payload
...

So, at least according to the docs, underlying tus-client-js library has mechanism to receive such check results, by providing lastResponse data.

However this event handler is explicitly overrided by Uppy omitting payload parameter for event and setting Body to {} explicitly:

uploadOptions.onSuccess = () => {
        const uploadResp = {
          uploadURL: upload.url ?? undefined,
          status: 200,
          body: {} as B,
        }
...

which looks like temporary hack gone wrong for me. It explicitly ignores available parametrs and returns hard-coded values instead of data provided by underlying library. I don't really see a point to return body and status fields at all, considering the fact that they are hard-coded values that have nothing to do with actual server response.

@ikopmans
Copy link
Author

ikopmans commented Sep 3, 2024

Just tried using onSuccess handler with tus-client-js directly, and in fact the tus-client-js documentation is wrong.
API docs say that onSuccess even handler receives some payload, bit in fact it does get nothing.

So, this issue makes no sense. There is no way for Uppy to get this data. Sorry for wasting your time. :(

@ikopmans ikopmans closed this as completed Sep 3, 2024
@Murderlon
Copy link
Member

You can open an issue in tus-js-client and once it's added there, we'll add it here.

@alexmv
Copy link

alexmv commented Sep 5, 2024

This functionality looks to have been added in tus/tus-js-client#689. Can you update Uppy to make that lastResponse available to the upload-success event?

@Murderlon Murderlon reopened this Sep 6, 2024
@Murderlon Murderlon changed the title response.body is always empty for upload-success event when using tus plugin @uppy/tus: add response to upload-success Sep 6, 2024
@Murderlon Murderlon self-assigned this Sep 6, 2024
@jarvis394
Copy link

I have been getting Tus response data from onAfterResponse method like this:

const handleUploadResponse = async (_req: HttpRequest, res: HttpResponse) => {
  try {
    // You can implement parsing with zod, yup, etc.
    const parsedBody = JSON.parse(res.getBody() || '') as TusHookResponse

    switch (parsedBody.type) {
      ...
    }
  } catch (e) {
    ...
  }
}

const [uppy] = useState(() =>
  new Uppy({ ... }).use(Tus, {
    onAfterResponse: handleUploadResponse,
  })
)
Types to make this safe

Typings

// Backend
export enum TusHookType {
  PRE_CREATE = 'pre-create',
  POST_CREATE = 'post-create',
  POST_RECEIVE = 'post-receive',
  PRE_FINISH = 'pre-finish',
  POST_FINISH = 'post-finish',
  POST_TERMINATE = 'post-terminate',
}

export type BaseTusHookResponseBody = {
  type: TusHookType
  ok: boolean
}

export type BaseTusHookResponseErrorBody = {
  type: TusHookType
  ok: false
  statusCode: number
  message: string
}

... // Inherit your responses from `BaseTusHookResponseBody`

export type TusHookResponse = TusHookPreCreateResponse | TusHookPreFinishResponse
// Respond to client with types above

TusHookResponse builder

export class TusHookResponseBuilder<
  ResBody extends Record<
    string,
    string | number | boolean
  > = BaseTusHookResponseBody,
> {
  private readonly data: TusHookResponse
  #body: ResBody = { ok: true } as unknown as ResBody

  get body(): ResBody {
    return this.#body
  }

  constructor() {
    this.data = {
      HTTPResponse: {
        StatusCode: 200,
        Body: JSON.stringify(this.body),
        Header: {
          'Content-Type': 'application/json',
        },
      },
    }
  }

  private stringifyBody() {
    this.data.HTTPResponse.Body = JSON.stringify(this.#body)
    return this
  }

  setStatusCode(code: number) {
    this.data.HTTPResponse.StatusCode = code
    return this
  }

  setBody(data: ResBody) {
    this.#body = data
    this.stringifyBody()
    return this
  }

  setBodyRecord(key: keyof ResBody, value: ResBody[keyof ResBody]) {
    this.#body[key] = value
    this.stringifyBody()
    return this
  }

  setRejectUpload(state: boolean) {
    this.data.RejectUpload = state
    return this
  }

  build(): TusHookResponse {
    return this.data
  }
}

Although, it would be heavily appreciated if Uppy client would emit response body in its events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants