apostrophe icon indicating copy to clipboard operation
apostrophe copied to clipboard

" redirect to the first prefixed locale that matches the requested hostname" test failures

Open eladavn opened this issue 10 months ago • 9 comments

To Reproduce

Step by step instructions to reproduce the behavior:

  1. Clone this repo
  2. npm install
  3. Start local mongodb daemon
  4. npx mocha test/i18n.js
  5. See error

Expected behavior

All tests pass

Describe the bug

Test output:

17 passing (7s) 2 failing

  1. redirection to first locale should redirect to the first prefixed locale that matches the requested hostname: FetchError: request to http://ca.localhost:37957/ failed, reason: getaddrinfo ENOTFOUND ca.localhost at ClientRequest. (node_modules/node-fetch/lib/index.js:1501:11) at ClientRequest.emit (node:events:517:28) at Socket.socketErrorListener (node:_http_client:501:9) at Socket.emit (node:events:517:28) at emitErrorNT (node:internal/streams/destroy:151:8) at emitErrorCloseNT (node:internal/streams/destroy:116:3) at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

  2. no redirection to first locale should not redirect to the first prefixed locale that matches the requested hostname when at least one locale has no prefix: FetchError: request to http://ca.localhost:36819/ failed, reason: getaddrinfo ENOTFOUND ca.localhost at ClientRequest. (node_modules/node-fetch/lib/index.js:1501:11) at ClientRequest.emit (node:events:517:28) at Socket.socketErrorListener (node:_http_client:501:9) at Socket.emit (node:events:517:28) at emitErrorNT (node:internal/streams/destroy:151:8) at emitErrorCloseNT (node:internal/streams/destroy:116:3) at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Details

Version of Node.js: v18.20.6

Server Operating System: Ubuntu 24.04.1 LTS on Windows wsl v2

Additional Context: Mongo v8.0.4

eladavn avatar Feb 14 '25 09:02 eladavn

Looking now.

BoDonkey avatar Feb 14 '25 10:02 BoDonkey

The failing tests are related to hostname-based localization testing. The tests try to access subdomains like ca.localhost to verify that the CMS redirects users to the correct locale based on the hostname. However, WSL2 can't resolve these test subdomains by default, which is why you're seeing the ENOTFOUND ca.localhost errors. The other tests pass because they only test path-based routing on regular localhost. You need to modify your hosts file - I don't do a lot with WSL2, but there should be a etc/hosts file that you can add something like:

127.0.0.1 ca.localhost
127.0.0.1 en.localhost

I was trying to find a decent link without a ton of ads!

BoDonkey avatar Feb 14 '25 11:02 BoDonkey

Is it currently working in "native" (not running within WSL) Ubuntu without any need for a modification?

eladavn avatar Feb 14 '25 11:02 eladavn

Nope. Doesn't work on MacOS without alteration of the hosts file either.

BoDonkey avatar Feb 14 '25 12:02 BoDonkey

Yes Bob is correct. Since testing the outcome with various hostnames is the point, you must edit your /etc/hosts file if you wish to run these tests yourself. Of course our CI system runs them in any case when we receive PRs etc.

These names must point to 127.0.0.1

On Fri, Feb 14, 2025, 7:50 AM Robert Means @.***> wrote:

Nope. Doesn't work on MacOS without alteration of the hosts file either.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/4862#issuecomment-2659261015, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27NFIX2UPEHFB5JU63L2PXRAVAVCNFSM6AAAAABXEGEQE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJZGI3DCMBRGU . You are receiving this because you are subscribed to this thread.Message ID: @.***> [image: BoDonkey]BoDonkey left a comment (apostrophecms/apostrophe#4862) https://github.com/apostrophecms/apostrophe/issues/4862#issuecomment-2659261015

Nope. Doesn't work on MacOS without alteration of the hosts file either.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/4862#issuecomment-2659261015, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27NFIX2UPEHFB5JU63L2PXRAVAVCNFSM6AAAAABXEGEQE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJZGI3DCMBRGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

boutell avatar Feb 14 '25 17:02 boutell

A much nicer approach IMHO would be to stop having the tests relying on external system configuration as this breaks unit tests isolation and requires special (undocumented?) modification to the host OS. This can be achieved by executing an in-process simple DNS server which would resolve these domain names to 127.0.0.1, then have the same process addressing that DNS server for name resolutions. To do that I suggest the following:

  1. Use dns2 for running an in-process DNS resolver configured to resolve en.localhost and ca.localhost to 127.0.0.1 . That resolver would listen for example on port 7777
  2. Call dns.setServers([127.0.0.1:7777]); , so every call made in that test process would first try to resolve the host through the server implemented in (1)

WDYT?

eladavn avatar Feb 15 '25 20:02 eladavn

Thanks for the suggestion. I appreciate the creative approach, but I have some concerns about adding an in-process DNS server for these tests. The issue we're seeing isn’t due to Apostrophe—it's simply standard DNS resolution where the system’s /etc/hosts file handles domain mapping. Relying on that is common practice and keeps the testing environment closer to a real-world setup.

Introducing a custom DNS resolver (with dns2 or similar) would add extra complexity and dependencies. This could make our tests more brittle and harder to debug, as well as increasing maintenance overhead. Given that modifying /etc/hosts is a straightforward, isolated change, it seems preferable to document this prerequisite rather than build additional infrastructure into our tests.

I hope this explains the rationale. Happy to discuss further if needed!

BoDonkey avatar Feb 16 '25 10:02 BoDonkey

Sure, that makes sense. I would at least make sure this pre-requisite is documented somewhere as part of the development environment setup.

eladavn avatar Feb 16 '25 10:02 eladavn

Yes that makes sense. It is only relevant for people who want to run the tests of Apostrophe itself, but third party contributors should get every assistance possible so they can contribute!

On Sun, Feb 16, 2025 at 5:52 AM eladavn @.***> wrote:

Sure, that makes sense. I would at least make sure this pre-requisite is documented somewhere as part of the development environment setup.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/4862#issuecomment-2661374446, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OCMN3WJMWDUUKTFAL2QBUXDAVCNFSM6AAAAABXEGEQE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRRGM3TINBUGY . You are receiving this because you commented.Message ID: @.***> [image: eladavn]eladavn left a comment (apostrophecms/apostrophe#4862) https://github.com/apostrophecms/apostrophe/issues/4862#issuecomment-2661374446

Sure, that makes sense. I would at least make sure this pre-requisite is documented somewhere as part of the development environment setup.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/4862#issuecomment-2661374446, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OCMN3WJMWDUUKTFAL2QBUXDAVCNFSM6AAAAABXEGEQE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRRGM3TINBUGY . You are receiving this because you commented.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell avatar Feb 16 '25 14:02 boutell