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

Fastify #9106

Merged
merged 42 commits into from
Dec 3, 2022
Merged

Fastify #9106

merged 42 commits into from
Dec 3, 2022

Conversation

syuilo
Copy link
Member

@syuilo syuilo commented Sep 27, 2022

What

Resolve #7537

Why

Additional info (optional)

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Sep 27, 2022
@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Nov 3, 2022
@syuilo
Copy link
Member Author

syuilo commented Nov 21, 2022

tasukete

@ycanardeau
Copy link

@syuilo I managed to fix this issue by extracting "actual" values from multipart fields (ycanardeau@e8fab4a), although I'm not sure if this is the right way to fix it.

const [path] = await createTemp();
await pump(multipartData.file, fs.createWriteStream(path));

const token = multipartData.fields['i'];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the multipartData.fields property returns an object like this:

{
  i: {
    fieldname: 'i',
    mimetype: 'text/plain',
    value: /* ... */,
    // ...
  },
  force: {
    fieldname: 'force',
    mimetype: 'text/plain',
    value: 'true',
    // ...
  },
  file: {
    fieldname: 'file',
    filename: 'blob',
    mimetype: /* ... */,
    file: /* ... */,
    // ...
  },
  name: {
    fieldname: 'name',
    mimetype: 'text/plain',
    value: /* ... */,
    // ...
  }
}

The extractMultipartFields method converts this object into the following format:

{
  i: /* ... */,
  force: 'true',
  file: undefined,
  name: /* ... */
}

However, one of the problems of the extractMultipartFields method is that it contains file: undefined, which does not exist on koa's equivalent. I'm not sure if this would be a problem though.

@ycanardeau
Copy link

Re: https://misskey.io/notes/989o0zrglu

@syuilo I was experiencing exactly the same issue on Windows 11 when I merged the latest commit from 7b563c1. I just deleted yarn.lock (and node_modules), executed yarn install, and then it worked. You may need to execute yarn add accepts on the packages/backend folder, because it's missing for some reason.

@syuilo
Copy link
Member Author

syuilo commented Dec 3, 2022

型が付かないのはなぜ?
image

@syuilo
Copy link
Member Author

syuilo commented Dec 3, 2022

ここも
image

@syuilo
Copy link
Member Author

syuilo commented Dec 3, 2022

tasukete

@syuilo syuilo marked this pull request as ready for review December 3, 2022 10:37
@syuilo syuilo merged commit 3a7182b into develop Dec 3, 2022
@syuilo syuilo deleted the fastify branch December 3, 2022 10:42
@tamaina
Copy link
Member

tamaina commented Dec 3, 2022

https://p1.a9z.dev/queue なくなった?

@syuilo
Copy link
Member Author

syuilo commented Dec 3, 2022

あー忘れてた

@tamaina
Copy link
Member

tamaina commented Dec 3, 2022

既知の問題: inboxがお亡くなりになってる

@syuilo
Copy link
Member Author

syuilo commented Dec 3, 2022

@tamaina
Copy link
Member

tamaina commented Dec 3, 2022

inboxはエラーも出てない

tamaina added a commit to tamaina/misskey that referenced this pull request Dec 3, 2022
nafu-at pushed a commit to TeamNijimiss/misskey that referenced this pull request Dec 3, 2022
@syuilo
Copy link
Member Author

syuilo commented Dec 4, 2022

@syuilo
Copy link
Member Author

syuilo commented Dec 4, 2022

@syuilo
Copy link
Member Author

syuilo commented Dec 4, 2022

てか逆にこれが効いたとすると、すべてのfastifyインスタンスのReplyにsendFileが生えてることになっちゃわない?(実際はプラグインを明示的にregisterしたインスタンスだけに生える)

@syuilo
Copy link
Member Author

syuilo commented Dec 4, 2022

:tasukete:

@syuilo
Copy link
Member Author

syuilo commented Dec 6, 2022

みす環かしら
fastify/fastify-cookie#224 (comment)

tamaina added a commit to tamaina/misskey that referenced this pull request Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

koaやめる
3 participants