Skip to content

fix: send non_splitting_tags and ignore_tags as arrays in JSON payloads#92

Merged
usox merged 1 commit intoSC-Networks:masterfrom
modrictin:master
Apr 8, 2025
Merged

fix: send non_splitting_tags and ignore_tags as arrays in JSON payloads#92
usox merged 1 commit intoSC-Networks:masterfrom
modrictin:master

Conversation

@modrictin
Copy link
Contributor

@modrictin modrictin commented Mar 26, 2025

🛠️ What this fixes:

We got a Http error occurred (400) error when migrating from 2.x to 4.0.

This PR corrects how non_splitting_tags, ignore_tags are handled when building the DeepL translation request body.

Previously, these were passed as comma-separated strings via:

implode(',', $this->translation->getNonSplittingTags())
implode(',', $this->translation->getIgnoreTags())

This worked when the request body was encoded as application/x-www-form-urlencoded, but the current implementation builds a JSON request body using a stream and sets the Content-Type to application/json.


✅ What this PR does:

  • Passes non_splitting_tags and ignore_tags as proper arrays in the JSON body (instead of comma-separated strings)
  • Ensures compatibility with DeepL’s stricter input validation on application/json payloads
  • Fixes unexpected Http error occurred (400) when setting non_splitting_tags or ignore_tags

🧪 Verified behavior:

We confirmed this fix by sending the following test input:

<par>The firm said it had been </par><par> conducting an internal investigation.</par>

With:

'non_splitting_tags' => ['par']

Result (correct behavior):

<par>Das Unternehmen teilte mit, dass es </par><par> eine interne Untersuchung durchgeführt habe.</par>

Without non_splitting_tags, DeepL treated the tags as sentence boundaries, leading to split context and degraded translation quality.

The same principle applies to ignore_tags, which must also be passed as an array when using JSON payloads.


⚠️ Important Note / Incomplete Fix:

This PR only addresses the issue in the DeeplTranslationRequestHandler.

However, the same incorrect handling of list-based parameters (via implode(...)) appears to be present in at least:

  • DeepBatchTranslationRequestHandler

There may be other request handlers or tests affected as well — I haven’t reviewed all of them.

🚧 This PR is not a full fix across the entire package. Contributions are welcome to complete this.

P.S. 'tag_handling' can be affected as well, but I don't think so.


✅ Summary:

This update brings the client in line with DeepL API expectations for JSON requests. It resolves a backend-breaking change and ensures future-proof compatibility with non_splitting_tags, ignore_tags, and other list-style fields.

Note: other request handlers (e.g., DeepBatchTranslationRequestHandler) and tests
may still contain similar issues. This commit does not address those.
@WorksDev WorksDev requested a review from a team March 26, 2025 15:33
@WorksDev WorksDev added the bug Something isn't working label Mar 26, 2025
@usox usox mentioned this pull request Apr 8, 2025
@usox
Copy link
Member

usox commented Apr 8, 2025

Hi @modrictin

Thank you for your contribution and the detailed description. This is looking good so far, we will have a look at the remaining payloads and will cover that within #93

@usox usox merged commit 065ba49 into SC-Networks:master Apr 8, 2025
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants