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

[useResponseCache] Promise rejections from cache impl are not handled #2240

Open
3 of 4 tasks
sachinahya opened this issue May 22, 2024 · 0 comments · May be fixed by #2241
Open
3 of 4 tasks

[useResponseCache] Promise rejections from cache impl are not handled #2240

sachinahya opened this issue May 22, 2024 · 0 comments · May be fixed by #2241
Labels
kind/bug Something isn't working stage/4-pull-request A pull request has been opened that aims to solve the issue

Comments

@sachinahya
Copy link

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

  • 1. The issue provides a
    minimal reproduction available on
    Stackblitz.
    • Please install the latest @envelop/* packages that you are using.
    • Please make sure the reproduction is as small as possible.
  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

The cache implementation can be sync or async - each of the methods can return a Promise. However, these promises are not awaited or handled so they end up as "floating promises". When one of these rejects, Node treats it as an unhandled promise rejection and, under default behaviour

To Reproduce Steps to reproduce the behavior:

  1. Set up response cache with graphql-yoga in Node.
  2. Override the cache with an implementation of set which returns a rejected promise.
  3. Execute a query that would be cached.
  4. The result returns since cache.set isn't awaited, but in the background the Node processes crashes.

We noticed this whilst using @envelop/response-cache-redis with a Redis setup that wasn't configured correctly, however, the incorrect error handling can be reproduced with anything that returns a rejected promise.

Expected behavior

The rejected promise should be returned through onExecuteDone so it can be handled by the server and result in a failed request. The process should not get terminated.

Environment:

  • OS: macOS 14.4.1
  • NodeJS: v20.11.0
  • @envelop/* versions:
    • @envelop/response-cache: 6.1.2
@sachinahya sachinahya linked a pull request May 22, 2024 that will close this issue
9 tasks
@EmrysMyrddin EmrysMyrddin added kind/bug Something isn't working stage/4-pull-request A pull request has been opened that aims to solve the issue labels May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working stage/4-pull-request A pull request has been opened that aims to solve the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants