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

API resolved without sending a response for /v2/file, this may result in stalled requests. #32

Open
rbabayoff opened this issue Aug 3, 2021 · 11 comments
Labels
good first issue Good for newcomers question Further information is requested

Comments

@rbabayoff
Copy link

Hey there,

Getting the message in the subject after every request in my dev env when running npm run dev.

How do I make it go away?

Thanks

@stegano
Copy link
Owner

stegano commented Aug 3, 2021

@rbabayoff
Copy link
Author

rbabayoff commented Aug 4, 2021

Hey, don't see how this is relevant. It seems unrelated to the proxy middleware. When trying to implement it as is I get an error that I cannot set headers after response has already been sent. This is the exact error message: I get when calling res.setHeader in the then clause:

httpProxyMiddleware error: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

@stegano
Copy link
Owner

stegano commented Aug 5, 2021

@rbabayoff The point of the answer above is to return a Promise.resolve() or Promise.reject().

@stegano stegano added good first issue Good for newcomers question Further information is requested labels Aug 5, 2021
@rbabayoff
Copy link
Author

@stegano just returning resolve or reject didn't work for me, I needed to also call res.end() before resolving or rejecting. Below's the entire code that worked for me, please let me know if that should be ok:

return new Promise<void>((resolve, reject) => {
  httpProxyMiddleware(req, res, {
    headers: {
      Authorization: "SomeAPIKey",
    },
  }).then(() => {
    res.end();
    resolve();
  }).catch((e) => {
    res.end();
    reject();
  });
});

Thanks

@stegano
Copy link
Owner

stegano commented Aug 16, 2021

@stegano just returning resolve or reject didn't work for me, I needed to also call res.end() before resolving or rejecting. Below's the entire code that worked for me, please let me know if that should be ok:

return new Promise<void>((resolve, reject) => {
  httpProxyMiddleware(req, res, {
    headers: {
      Authorization: "SomeAPIKey",
    },
  }).then(() => {
    res.end();
    resolve();
  }).catch((e) => {
    res.end();
    reject();
  });
});

Thanks

Hi rbabayoff

There seems to be no problem in operation even with this code.
However, it seems that an asynchronous function is created and processed unnecessarily.

Are the symptoms still present? In my case, the issue is not reproduced as a result of running the sample code below.

export default (req: NextApiRequest, res: NextApiResponse) => (
  httpProxyMiddleware(req, res, {
    headers: {
      Authorization: 'TEST',
    },
    // You can use the `http-proxy` option
    target: 'http://127.0.0.1:8080',
    // In addition, you can use the `pathRewrite` option provided by `next-http-proxy`
    pathRewrite: {
      '^/api/new': '/v2',
      '^/api': '',
    },
  })
);

I checked this library code again, and it was already returning Promise.resolve and Promise.reject internally.
(Note: https://github.com/stegano/next-http-proxy-middleware/blob/master/src/index.ts#L65L66)

Thanks :)

@rametta
Copy link

rametta commented Nov 22, 2021

Hello, I'm also experiencing this warning show up and it seems like it's related to this package. I've tried implementing the suggestion from @rbabayoff but that actually caused more problems. So I'm not sure how to get rid of these warnings/errors, and if they're false positive or not.

@stegano
Copy link
Owner

stegano commented Nov 23, 2021

Hi @rametta
Could you show me the code using next-http-proxy-middleware?

@JackCuthbert
Copy link

JackCuthbert commented Feb 7, 2022

Struggled with this today (even went as far as cloning and trying to debug the package itself), hoping that I can clear this up for others having this issue.

This warning has no functional impact on the proxy and it's responses, it is a "warning" after all. It looks like it has something to do with the way Next checks that their API route URLs resolve anything, the path rewrite happening during the proxy (URLs change when you rewrite them 🤯), or simply having the proxy resolve the request rather than Next. They have an escape hatch for exactly this kind of problem too! Check out: externalResolver

This flag tells Next that the API is resolved externally to the built-in API route and resolvers. Check docs here.

Altogether this looks like...

// ./pages/api/[...proxy].ts
export const config = {
  api: {
    externalResolver: true
  }
}

export default (req, res) => httpProxyMiddleware(req, res, {
  target: 'http://localhost:1337,
  pathRewrite: [
    {
      patternStr: '^/api',
      replaceStr: ''
    }
  ]
})

@stegano
Copy link
Owner

stegano commented Feb 9, 2022

@JackCuthbert Thanks for the great answer! I'll add it to the documentation.

@stegano
Copy link
Owner

stegano commented Feb 13, 2022

@all-contributors please add @JackCuthbert for doc

@allcontributors
Copy link
Contributor

@stegano

I've put up a pull request to add @JackCuthbert! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants