discordjs / discord.js

A powerful JavaScript library for interacting with the Discord API
https://discord.js.org
Apache License 2.0
25.45k stars 3.97k forks source link

Creating a Sticker with an apng file does not work - `DiscordAPIError[50046] Invalid Asset` #8557

Closed SethCohen closed 1 year ago

SethCohen commented 2 years ago

Which package is this bug report for?

discord.js

Issue description

After updating from v13 to v14, I've noticed that one of my previous commands no longer works. Uploading an animated png file now throws this such error:

DiscordAPIError[50046]: Invalid Asset
    at SequentialHandler.runRequest (/home/sethdev/WebstormProjects/EmojiUtilities/node_modules/@discordjs/rest/dist/lib/handlers/SequentialHandler.cjs:293:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async SequentialHandler.queueRequest (/home/sethdev/WebstormProjects/EmojiUtilities/node_modules/@discordjs/rest/dist/lib/handlers/SequentialHandler.cjs:99:14)
    at async REST.request (/home/sethdev/WebstormProjects/EmojiUtilities/node_modules/@discordjs/rest/dist/lib/REST.cjs:52:22)
    at async GuildStickerManager.create (/home/sethdev/WebstormProjects/EmojiUtilities/node_modules/discord.js/src/managers/GuildStickerManager.js:67:21)
    at async /home/sethdev/WebstormProjects/EmojiUtilities/src/commands/stickerfy.js:108:7 {
  rawError: { message: 'Invalid Asset', code: 50046 },
  code: 50046,
  status: 400,
  method: 'POST',
  url: 'https://discord.com/api/v10/guilds/779897232295329793/stickers',
  requestBody: {
    files: [ [Object] ],
    json: { name: 'test', tags: 'banana', description: '' }
  }
}

Steps to reproduce with below code sample:

  1. Create an animated png file
  2. Input it into guild.stickers.create()
  3. Watch it fail

Interestingly enough, it works if the supplied file is an non-animated png or if the file is a URL (A URL apng and png both create a sticker perfectly fine). I've checked the magic bytes of several non-animated and animated png files and they are both recognized as pngs; animated png vs non-animated png

My current suspicions of the previous fixes from #8285 and #8290 didnt actually implement a fix for apngs and dont properly assign the png content-type to apngs, maybe?

For replication testing purposes, here is an animated png: https://i.imgur.com/vZbdGUW.png Feel free to try uploading it as a sticker via Discord manually, via the guild.stickers.create() function as a URL, and then again via as a file. The first two should succeed, whilst the last fails.

Code sample

guild.stickers.create({ file: './testimage.png', name: 'test', tags: 'banana' })
.then(console.log)
.catch(console.error)

Package version

14.3.0

Node.js version

18.7.0

Operating system

Arch Linux

Priority this issue should have

Medium (should be fixed soon)

Which partials do you have configured?

User, Channel, Message, Reaction

Which gateway intents are you subscribing to?

Guilds, GuildEmojisAndStickers, GuildMessages, GuildMessageReactions, MessageContent

I have tested this issue on a development release

No response

didinele commented 2 years ago

Could you mess around with file-type (NPM) yourself and see what the recognized mime type is? I have a suspicion the fault arises from here: https://github.com/discordjs/discord.js/blob/0674820723ac9fe57cdd85acdd164a8a2305ea6d/packages/rest/src/lib/RequestManager.ts#L417

Especially considering that you mentioned it works fine with URLs (in which case we take the response Content-Type header as the mime type and it's passed down from discord.js itself)

SethCohen commented 2 years ago

Could you mess around with file-type (NPM) yourself and see what the recognized mime type is? I have a suspicion the fault arises from here:

https://github.com/discordjs/discord.js/blob/0674820723ac9fe57cdd85acdd164a8a2305ea6d/packages/rest/src/lib/RequestManager.ts#L417

Especially considering that you mentioned it works fine with URLs (in which case we take the response Content-Type header as the mime type and it's passed down from discord.js itself)

With using

import {fileTypeFromFile, fileTypeFromBuffer} from 'file-type';
import {readChunk} from 'read-chunk';
import fs from 'fs';

console.log(await fileTypeFromFile('testimage.png'));

const buffer = await readChunk('testimage.png', { length: 4100 });
console.log(await fileTypeFromBuffer(buffer));

const buffer2 = fs.readFileSync('testimage.png');
console.log(await fileTypeFromBuffer(buffer2));

All three returned

{ ext: 'apng', mime: 'image/apng' }
{ ext: 'apng', mime: 'image/apng' }
{ ext: 'apng', mime: 'image/apng' }

Could the issue be due to it not being image/png?

didinele commented 2 years ago

Could the issue be due to it not being image/png?

It... sure sounds like it, which would make this an API bug(?), probably.

Interestingly enough, it works [...] if the file is a URL [...]

Could you now fetch the files you were testing with and check what res.headers.get('content-type') is?

SethCohen commented 2 years ago

Could the issue be due to it not being image/png?

It... sure sounds like it, which would make this an API bug(?), probably.

Interestingly enough, it works [...] if the file is a URL [...]

Could you now fetch the files you were testing with and check what res.headers.get('content-type') is?

Wasn't too sure what you meant by fetching the files, but testing DataResolver.resolveFile() with some apng URLs returned a content-type of image/png .

console.log(await DataResolver.resolveFile('https://i.imgur.com/vZbdGUW.png'));

didinele commented 2 years ago

That works too. Thanks! I'll try to flag this as an API bug and if that doesn't work we'll need to hack this up internally, but I hope they take it.

SethCohen commented 2 years ago

I've managed to fix my issue on my side by changing RequestManager locally from

const contentType = file.contentType ?? (await fileTypeFromBuffer(file.data))?.mime;

to

const fileMime = (await fileTypeFromBuffer(file.data))?.mime
const contentType = file.contentType ?? fileMime === 'image/apng' ? 'image/png' : fileMime;

Which is a bit jank, in my personal opinion, haha, but it temporarily gets the job done for anyone else who may be having issues with uploading file-based apng stickers.

I don't particularly want to make an actual PR as their may be better alternatives, but considering the file-type package distinguishes apngs vs pngs as different media types, this may be one of the simplest solutions; simply just changing the media type from image/apng back to image/png in consideration that normally most systems do not often distinguish the two png types as different media types in the first place.

It might be in merit to consider either a different dependency, or allowing/not throwing an error on content-type image/apng, etc. I'll leave it up to someone else to make an actual PR though to either implement this solution, or propose a more reasonable solution - if there are any.

didinele commented 2 years ago

It might be in merit to consider either a different dependency

No, using a different dependency that yields an (arguably) less correct MIME type is backwards.

or allowing/not throwing an error on content-type image/apng

This isn't an error we're throwing, its just what Discord is throwing.

We'll end up having some logic take care of those special cases if Discord doesn't accept my issue as a bug, but I haven't opened that yet as it's the weekend and I wanted to relax, I'll be doing it tomorrow. Ideally they do allow us to pass image/apng, in which case no code changes on our end will be needed.

advaith1 commented 2 years ago

As the official list does not include image/apng, the actual issue here appears to be file-type returning a non-standard type string.