8000 [Translation] Push and pull translations commands say OK even no translations were transmitted · Issue #45920 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Translation] Push and pull translations commands say OK even no translations were transmitted #45920

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

Closed
ossinkine opened this issue Apr 2, 2022 · 2 comments

Comments

@ossinkine
Copy link
Contributor

Symfony version(s) affected

=5.4

Description

I looked at the providers behavior and noticed that in some cases, if the provider API did not respond with a 200 HTTP code, an exception is thrown, and in other cases, it just add the record in the log.

This means that if the provider API responded with for example 503 code (was under maintenance) or any other 5xx code when creating translations, no translations will be pushed, but the translation:push command says [OK] New local translations has been sent.

We use Crowdin, but looks like other providers have the same behavior.

How to reproduce

You need to fake the provider API response, like if there was a 500 response here. No translations will be uploaded but command ends successfully.

Possible Solution

I suggest to throw an provider exception every time when HTTP request ends unexpected and action is not performed.

Additional Context

No response

@welcoMattic
Copy link
Member

@ossinkine Thanks for reporting this issue. Actually yes, we should throw an exception if any 5xx error code is returned for any HTTP request in any Provider.

Feel free to open a PR to fix the problem. If you need help, don't hesitate to ask for it

@fabpot fabpot closed this as completed Aug 9, 2022
fabpot added a commit that referenced this issue Aug 9, 2022
… is 50x (alamirault)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Translation] Crowdin provider throw Exception when status is 50x

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #45920 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

When calling provider return server error, it was silently accepted (with warning log in some cases).
Now, `ProviderException` is thrown. This avoid `[OK] New local translations has been sent` when there is one failed call.

Only Crowdin provider is changed here. If it's right approach, I can edit this PR or create new one foreach provider

Commits
-------

3b7aed2 [Translation] Crowdin provider throw Exception when status is 50x
@d47081
Copy link
d47081 commented Oct 3, 2023

I have same issue, just debug returns 200 code in my case

php bin/console translation:push crowdin -vvv
02:46:14 INFO      [http_client] Request: "GET https://api.crowdin.com/api/v2/projects/..."
02:46:15 INFO      [http_client] Response: "200 https://api.crowdin.com/api/v2/projects/..."
02:46:15 INFO      [http_client] Request: "GET https://api.crowdin.com/api/v2/languages?limit=500"
02:46:15 INFO      [http_client] Response: "200 https://api.crowdin.com/api/v2/languages?limit=500"
02:46:15 INFO      [http_client] Request: "GET https://api.crowdin.com/api/v2/projects/..."
02:46:15 INFO      [http_client] Response: "200 https://api.crowdin.com/api/v2/projects/..."

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

No branches or pull requests

5 participants
0