proxyReq behavior change? Now only runs for values matched in `proxies` config.

brian428 commented 6 years ago

Expected behaviour

If a proxyReq function is set, it should run for all requests, not just requests that match paths defined in proxies config object.

Actual behaviour

This worked as expected (proxyReq always ran, if defined) prior to upgrading to Angular 6 / Angular CLI 6. Now, proxyReq only seems to run if one or more paths are set in proxies, and only runs for paths that match those defined in proxies. I'm not sure when this changed (2.0?), or if it is actually related to the version of node-http-proxy being used. (Edit: I'm on 2.0.2, so I don't think this change could be the culprit, but just in case: https://github.com/karma-runner/karma/commit/0dc8ea4ab5d281e62d324ab02bc79acd1bdd38b1#diff-a043d5a6a7b3405edd843f08f24e3da1)

Environment Details

Karma version (output of karma --version): 2.0.2
Relevant part of your karma.config.js file

proxyReq: function( clientRequest, incomingMessage, serverResponse, options ) {
  console.log( "Karma proxyReq function called." );
},

Steps to reproduce the behaviour

Set up a proxyReq function in the Karma config, but define no proxies.
Run Karma and note that proxyReq function never runs
Add a path to proxies that will not be used during your Karma run
Run Karma, and note that proxyReq is still not called
Add a path to proxies that will be used during your Karma run
Run Karma, and note that proxyReq is now called

Justification

Our app conditionally uses a set of mock JSON files to simulate a back-end server (based on runtime configuration). We find this very useful, for many reasons:

App can be run without any back-end server
App can be run with a back-end server, but endpoints that are not ready can be stubbed with a mock JSON file, so UI work can continue while server work is in progress
Tests can also use the mock JSON data. This avoids having to painstakingly add manual mock server results to all tests, and ensures that the tests use exactly the same data that users see if they run the app without a server.

All that leads to a simple issue: many app server calls are not GET requests, and non-GET requests will fail when the target is a static JSON file. We use the proxyReq to change the request method from POST/PUT/DELETE to GET, so the calls can proceed without issue.

Once I figured out that a proxies entry was required, I tried simply doing this to get the proxyReq to run:

proxies: {
  '/assets/mockdata/': '/assets/mockdata/'
},

But this just triggers an infinite loop in the handling of the request. The only alternative is to use a placeholder path for the mock JSON URLs, and use proxyReq to rewrite the path in addition to changing the request method.

Ideally, proxyReq would run whether any proxies are defined or not. Or, in lieu of that, provide some way for a path in proxies to trigger proxyReq, while indicating that the target for the proxy path should just remain the same as the original path and without triggering an infinite loop.

johnjbarton commented 6 years ago

Please try to find the last version that this worked, between v2.0.4 (current) and v1.7.1.

brian428 commented 6 years ago

Actually, it looks like it worked as recently as Karma 2.0.0. Not sure if that helps or not, since the commit I found (above) to /middleware/proxy.js was very recent. Could something else have changed between 2.0.0 and 2.0.2 that might have caused this?

johnjbarton commented 6 years ago

We've been doing quite a lot of refactoring and sometimes tests don't cover every case. So yes ;-)

karma-runner / karma