Skip to content

[Notifier] [Twitter] Fix post INIT upload #58658

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 9, 2024
Merged

[Notifier] [Twitter] Fix post INIT upload #58658

merged 1 commit into from
Nov 9, 2024

Conversation

matyo91
Copy link
Contributor

@matyo91 matyo91 commented Oct 25, 2024

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix Twitter Notifier when attaching a media.
License MIT

The procedure is described here : https://developer.x.com/en/docs/x-api/v1/media/upload-media/api-reference/post-media-upload-init. This is tricky as on example Example Request the media_type query parameter value is raw encoded : image/jpeg or video/mp4 for exemple.

On Postman, query param value is raw encoded too on the interface, but with PHP CURL generated, it's Percent encoded or rawurl encoded : video%252Fmp4. I did test the CURL generation and it was OK. But it's not the case on Twitter Notifier component that produce [Symfony\Component\Notifier\Exception\TransportException (32)] Could not authenticate you (so this fix).

postman

It did several tests, first I think it was the OAuth, but looking at the code on Symfony\Component\Notifier\Bridge\Twitter\TwitterTransport::request method and twitter docs, it was fine by comparing the php curl generated by postman and the curl generated by Symfony\Component\HttpClient\CurlHttpClient.

The difference was on the query (look at media_type param that is raw). This one without the fix produce the Exception.

[1:27:06][math@mathieus-mbp ~/Sites/darkwood/flow-live] (wave-function-collapse)$ bin/console app:wave-function-collapse -vvv
23:27:44 INFO      [http_client] Request: "POST https://upload.twitter.com/1.1/media/upload.json?command=INIT&total_bytes=56740&media_type=video/mp4&media_category=tweet_video"
23:27:44 INFO      [http_client] Response: "401 https://upload.twitter.com/1.1/media/upload.json?command=INIT&total_bytes=56740&media_type=video/mp4&media_category=tweet_video"

In TwitterTransport.php line 247:
                                                                  
  [Symfony\Component\Notifier\Exception\TransportException (32)]  
  Could not authenticate you                                      
                                                                  

Exception trace:
  at /Users/math/Sites/darkwood/flow-live/vendor/symfony/twitter-notifier/TwitterTransport.php:247
 Symfony\Component\Notifier\Bridge\Twitter\TwitterTransport->processChunk() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/twitter-notifier/TwitterTransport.php:198
 Symfony\Component\Notifier\Bridge\Twitter\TwitterTransport->uploadMedia() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/twitter-notifier/TwitterTransport.php:120
 Symfony\Component\Notifier\Bridge\Twitter\TwitterTransport->doSend() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/notifier/Transport/AbstractTransport.php:80
 Symfony\Component\Notifier\Transport\AbstractTransport->send() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/notifier/Transport/Transports.php:74
 Symfony\Component\Notifier\Transport\Transports->send() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/notifier/Chatter.php:46
 Symfony\Component\Notifier\Chatter->send() at /Users/math/Sites/darkwood/flow-live/src/Command/WaveFunctionCollapseCommand.php:61
 App\Command\WaveFunctionCollapseCommand->execute() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/console/Command/Command.php:279
 Symfony\Component\Console\Command\Command->run() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/console/Application.php:1047
 Symfony\Component\Console\Application->doRunCommand() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/framework-bundle/Console/Application.php:123
 Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/console/Application.php:316
 Symfony\Component\Console\Application->doRun() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/framework-bundle/Console/Application.php:77
 Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/console/Application.php:167
 Symfony\Component\Console\Application->run() at /Users/math/Sites/darkwood/flow-live/vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php:49
 Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner->run() at /Users/math/Sites/darkwood/flow-live/vendor/autoload_runtime.php:29
 require_once() at /Users/math/Sites/darkwood/flow-live/bin/console:15

app:wave-function-collapse [--width [WIDTH]] [--height [HEIGHT]] [--dataset [DATASET]]

And this one with the fix produce is OK. (look at media_type param that is rawurl encoded)

[0:11:12][math@mathieus-mbp ~/Sites/darkwood/flow-live] (wave-function-collapse)$ bin/console app:wave-function-collapse -vvv
22:11:18 INFO      [http_client] Request: "POST https://upload.twitter.com/1.1/media/upload.json?command=INIT&total_bytes=56740&media_type=video%252Fmp4&media_category=tweet_video"
22:11:18 INFO      [http_client] Response: "202 https://upload.twitter.com/1.1/media/upload.json?command=INIT&total_bytes=56740&media_type=video%252Fmp4&media_category=tweet_video"
22:11:18 INFO      [http_client] Request: "POST https://upload.twitter.com/1.1/media/upload.json?command=APPEND&media_id=1849574415295537152&segment_index=0"
22:11:18 INFO      [http_client] Response: "204 https://upload.twitter.com/1.1/media/upload.json?command=APPEND&media_id=1849574415295537152&segment_index=0"
22:11:18 INFO      [http_client] Request: "POST https://upload.twitter.com/1.1/media/upload.json?command=FINALIZE&media_id=1849574415295537152"
22:11:19 INFO      [http_client] Response: "200 https://upload.twitter.com/1.1/media/upload.json?command=FINALIZE&media_id=1849574415295537152"
22:11:19 INFO      [http_client] Request: "GET https://upload.twitter.com/1.1/media/upload.json?command=STATUS&media_id=1849574415295537152"
22:11:20 INFO      [http_client] Response: "200 https://upload.twitter.com/1.1/media/upload.json?command=STATUS&media_id=1849574415295537152"
22:11:20 INFO      [http_client] Request: "POST https://api.twitter.com/2/tweets"
22:11:20 INFO      [http_client] Response: "201 https://api.twitter.com/2/tweets"

To reproduce it :

/** @var ChatterInterface $chatter */
$videoFile = new \Symfony\Component\Mime\Part\File('var/cache/dev/wave_function_collapse/wave_function_collapse_6716bd83ad525.mp4');
$message = (new ChatMessage('Daily Flow generation.', (new TwitterOptions())->attachVideo($videoFile)))->transport('twitter');
$chatter->send($message);

My tests were only with mp4 file. I didn't test with gif or media_category = 'subtitles' but I assume it's the same logic.
It was a real test done with a tweeter account, so can't really reproduce it without creating your own keys Twitter notifier docs with read and write Twitter Access Token + Secret with a twitter account.

@matyo91 matyo91 requested a review from OskarStark as a code owner October 25, 2024 00:08
@carsonbot carsonbot added this to the 6.4 milestone Oct 25, 2024
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 7.2 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@carsonbot carsonbot changed the title [Twitter][Notifier] Fix rawurlencode media_type [Notifier] [Twitter] Fix rawurlencode media_type Oct 25, 2024
@matyo91
Copy link
Contributor Author

matyo91 commented Oct 25, 2024 •
edited
Loading

As it is a POST request, on the second commit, I updated the query params to body params that do the same without using rawurlencode. I will rebase the branch, when PR is validated, or choose the commit you prefer.

nicolas-grekas
nicolas-grekas reviewed Nov 4, 2024
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM with $body. Please update the PR/commit title then.

@matyo91 matyo91 changed the title [Notifier] [Twitter] Fix rawurlencode media_type [Twitter][Notifier] Fix post INIT upload Nov 6, 2024
@matyo91 matyo91 force-pushed the twitter-notifier branch 2 times, most recently from 2b512f1 to cae2e8e Compare November 6, 2024 18:45
@carsonbot carsonbot changed the title [Twitter][Notifier] Fix post INIT upload [Notifier] [Twitter] Fix post INIT upload Nov 9, 2024
@fabpot
Copy link
Member

fabpot commented Nov 9, 2024

Thank you @matyo91.

@fabpot fabpot merged commit 6fb5163 into symfony:6.4 Nov 9, 2024
9 of 10 checks passed
@matyo91 matyo91 deleted the twitter-notifier branch November 9, 2024 12:57
This was referenced Nov 13, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark Awaiting requested review from OskarStark OskarStark is a code owner

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

Assignees
No one assigned
Labels
Bug Notifier Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
4 participants
@matyo91 @carsonbot @fabpot @nicolas-grekas