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

Translator Option 'serverUrl' is misleading #46

Open
jblossey opened this issue May 20, 2024 · 1 comment
Open

Translator Option 'serverUrl' is misleading #46

jblossey opened this issue May 20, 2024 · 1 comment

Comments

@jblossey
Copy link

jblossey commented May 20, 2024

Describe the bug

According to RFC1738 a HTTP URL takes the form of http://<host>:<port>/<path>?<searchpart>.

When setting the serverUrl in the TranslatorOptions, it only replaces the host:port part.

To Reproduce

Construct a new Translator:

const translator = new deepl.Translator(
      process.env.DEEPL_API_KEY,
      {
          serverUrl: 'http://some-other-host.com/resource',
        },
    );

Translate something through this translator and the server at http://some-other-host.com will receive POST /resource/v2/translate HTTP/1.1.

Expected behavior

The option should replace the whole URL and not add its own path to the request.
Alternatively, it should be renamed to use the proper terminology, e.g. replaceHostAndPort.

@JanEbbing
Copy link
Member

JanEbbing commented May 21, 2024

Hi, thanks for the report.

When setting the serverUrl in the TranslatorOptions, it only replaces the host:port part.

This is incorrect, as your example shows (/resource is included in the final URL). We just also append the correct endpoint, as our API has separate endpoints for separate functionality (text translation, document translation, querying usage, etc). So, what you ask for (being able to set the final server URL) is not possible, you need to point it at a URL where the API endpoints are hosted. This can then be used with our mock server, for example.

I agree we can find a clearer name like setApiBase or similar, but I'm not sure it's worth making a breaking change for this (and having both seems unnecessary and confusing).

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

No branches or pull requests

2 participants