Skip to content

proxy.bypass return false does not work #1677

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

Closed
1 of 2 tasks
jsmith-sapient opened this issue Feb 21, 2019 · 15 comments · Fixed by #1696
Closed
1 of 2 tasks

proxy.bypass return false does not work #1677

jsmith-sapient opened this issue Feb 21, 2019 · 15 comments · Fixed by #1696

Comments

@jsmith-sapient
Copy link

  • Operating System: MacOS High Sierra
  • Node Version: 11.0.0
  • NPM Version: 6.4.1
  • webpack Version: 4.29.5
  • webpack-dev-server Version: 3.2.0
  • This is a bug
  • This is a modification request

Code

does not work:

// webpack.config.js

module.exports = {
  devServer: {
    proxy: {
      '**': {
        bypass(req) {
          if (!req.headers.accept.includes('application/json'))
            return false; // ← does nothing
        },
        target: `http://localhost:${yargs.proxyPort}`,
      },
    },
  },
  entry: './app',
}

does work:

// webpack.config.js

module.exports = {
  devServer: {
    proxy: {
      '**': {
        bypass(req) {
          if (!req.headers.accept.includes('application/json'))
            return req.url;
        },
        target: `http://localhost:${yargs.proxyPort}`,
      },
    },
  },
  entry: './app',
}

Expected Behaviour

Based on the vague explanation in the docs

It must return either false or a path that will be served instead of continuing to proxy the request.

I expect return false to skip the proxy

Actual Behaviour

return false does nothing.

For Bugs; How can we reproduce the behaviour?

Minimal example: https://github.com/jsmith-sapient/local-api-mocker

npm run start:dev
@alexander-akait
Copy link
Member

Can you try return null or something else? Looks like regression in http-proxy/http-proxy-middleware

@alexander-akait
Copy link
Member

Anyway PR welcome

@jsmith-sapient
Copy link
Author

@evilebottnawi I tried null: No difference to false (it does nothing).

I'll try to find some time this weekend to take a look. I might be posting as @jshado1 (my personal account, not a catfish).

@alexander-akait
Copy link
Member

@jsmith-sapient i will look on this in near future, anyway we glad any help

@alexander-akait
Copy link
Member

@jsmith-sapient reproducible test repo works fine, i am use /test and it is return json. What expected?

@jsmith-sapient
Copy link
Author

jsmith-sapient commented Feb 21, 2019

@alexander-akait
Copy link
Member

alexander-akait commented Feb 21, 2019

@jsmith-sapient thanks, it is very strange because we have tests on this case and tests passed 😕

@jsmith-sapient
Copy link
Author

jsmith-sapient commented Feb 21, 2019

@evilebottnawi could you link me to the test case? that might help. nvmd, it was easy to find: https://github.com/webpack/webpack-dev-server/blob/master/test/Proxy.test.js#L107-L119

@jsmith-sapient
Copy link
Author

@evilebottnawi I think you're talking about this case:

it('should pass through a proxy when a bypass function returns null', (done) => {
  req.get('/foo.js').expect(200, /Hey/, done);
});

If so, the function does not return null:

  '/foo': {
    bypass(req) {
      if (/\.html$/.test(req.path)) {
        return '/index.html';
      }
    },
  },

@alexander-akait
Copy link
Member

@jsmith-sapient we return nothing in this test, i think null, undefined, false is same in this context 😕

@jsmith-sapient
Copy link
Author

I don't think so. I would expect no return to mean "go ahead, proxy, do your thing". How else would that be signalled?

@alexander-akait
Copy link
Member

@jsmith-sapient in previous versions all works fine, right?

@JakobJingleheimer
Copy link

I haven't used bypass with previous versions.

@jsmith-sapient
Copy link
Author

I'm having trouble finding where this is in the source code. Could you point me in the right direction?

@alexander-akait
Copy link
Member

/cc @hiroppy need fix this problem next 👍 Should be not difficult

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

Successfully merging a pull request may close this issue.

3 participants