deepl-node icon indicating copy to clipboard operation
deepl-node copied to clipboard

Translator Option 'serverUrl' is misleading

Open jblossey opened this issue 1 year ago • 1 comments

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.

jblossey avatar May 20 '24 15:05 jblossey

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).

JanEbbing avatar May 21 '24 08:05 JanEbbing