Responses from filesUploadV2 have files nested in the files array

[zimeg]

Overview

A successful response from uploading files with the filesUploadV2 API contains a files attribute after a request like so:

const result = await web.filesUploadV2({
  token: "xoxp-example",
  channel_id: "C0123456789",
  file: "hello",
  filename: "greetings.md",
});

{
  ok: true,
  files: [ { ok: true, files: [Array], response_metadata: [Object] } ]
}

Which means accessing the ID of this file needs:

console.log(result.files[0].files[0].id);

The first [0] index is also consistent when multiple files are uploaded:

const result = await web.filesUploadV2({
  token: "xoxp-example",
  channel_id: "C0123456789",
  file_uploads: [
    {
      file: "README.md",
      filename: "README.md",
    },
    {
      content: "hello",
      filename: "greetings.md",
    },
  ],
});

console.log(result.files[0].files[0].id); // F00000README
console.log(result.files[0].files[1].id); // F00GREETINGS

Expected behaviors

This seems like unexpected behavior so wanted to check - it caught me by surprise! I expected result.files[0].id and result.files[1].id to have the file IDs 🤔

I found the following quick change removes the middle files[0] too:

node-slack-sdk/packages/web-api/src/WebClient.ts

Line 454 in 1938408

return { ok: true, files: completion };

-   return { ok: true, files: completion };
+   return {
+     ok: true,
+     files: completion[0].files as FilesGetUploadURLExternalResponse[],
+     response_metadata: completion[0].response_metadata,
+   };

Also noticing a few oddities around typing differences between filesUploadV2 and files.uploadV2 but this doesn't seem so important to me if filesUploadV2 is the recommended method of file uploads (typings work well here!).

Packages:

Select all that apply:

• @slack/web-api

Reproducible in:

The Slack SDK version

@slack/[email protected]
@slack/[email protected]

Node.js runtime version

v20.12.2

OS info

ProductName:            macOS
ProductVersion:         14.5
BuildVersion:           23F79
Darwin Kernel Version 23.5.0: Wed May  1 20:12:58 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6000

Requirements

For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. 🙇

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

[filmaj]

Oh weird. Yeah that doesn't seem right 😬 Oh wait.. there's a difference between web.files.uploadV2 and web.filesUploadV2 isn't there?

Damn, I don't think I actually audited web.filesUploadV2 at all.. I just looked at the methods within web.files.*... yeah, I didn't write up any migration instructions for v7 for web.filesUploadV2, just for web.files.uploadV2 😢

This is likely an oversight by me as part of the work to release web-api v7.

[zimeg]

@filmaj no blame at all since this wasn't covered by test! I've even poked this method since that release and didn't catch this until now!

The difference between web.files.uploadV2 and web.filesUploadV2 also surprised me. Without digging into it, it's not so clear to me what web.files.uploadV2 actually maps to 🤔 It might make sense to only support the wrapping web.filesUploadV2 here IMO 😳

If this does seem unexpected, with the top level result.files[ii] containing actual file information being correct, does it seem right to mark this as a semver:major fix?

[filmaj]

Yes, unfortunately :(

[zimeg]

Ahh this might be a nice reminder that versioning signals the type of change being made and nothing more, nothing too deep to read into 😰 This is what I'm telling myself.

[filmaj]

Yeah it all depends on the project. web-api doesn't tend to do majors often, so naturally the amount of breaking changes add up over time, so we kind of do it to ourselves.

I steward a different open source project that releases major new versions more frequently - maybe two or three times a year - but the backwards incompatibilities are minor and few and far between that most folks are unaffected by upgrading. Top it off with some good upgrade instructions with each major release and it's not actually that painful or scary.

I compare it to testing and CI: the less often you test, the more changes batch up, the harder it becomes to test - a negative feedback loop.

[github-actions]

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized.

[zimeg]

📝 Per threaded suggestions from the great @filmaj I've added this to the @slack/types@v3 since plans are pointing towards solid improvements in handling here that can be applied to a fix!