Skip to content
This repository was archived by the owner on Nov 23, 2022. It is now read-only.

Exception thrown when rate limit exceeded #1

Closed
maayoko opened this issue Jul 5, 2019 · 10 comments
Closed

Exception thrown when rate limit exceeded #1

maayoko opened this issue Jul 5, 2019 · 10 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@maayoko
Copy link

maayoko commented Jul 5, 2019

Hi, I'm getting exception thrown when rate limiter is exceeded.

 /usr/src/app/node_modules/rxjs/internal/util/hostReportError.js:4
     setTimeout(function () { throw err; }, 0);
                              ^
 
 TypeError: You provided an invalid object where a stream was expected. You can provide an Observable, Promise, Array, or Iterable.
     at Object.exports.subscribeTo (/usr/src/app/node_modules/rxjs/internal/util/subscribeTo.js:29:15)
     at Object.subscribeToResult (/usr/src/app/node_modules/rxjs/internal/util/subscribeToResult.js:14:26)
     at MergeMapSubscriber._innerSub (/usr/src/app/node_modules/rxjs/internal/operators/mergeMap.js:82:29)
     at MergeMapSubscriber._tryNext (/usr/src/app/node_modules/rxjs/internal/operators/mergeMap.js:76:14)
     at MergeMapSubscriber._next (/usr/src/app/node_modules/rxjs/internal/operators/mergeMap.js:59:18)
     at MergeMapSubscriber.Subscriber.next (/usr/src/app/node_modules/rxjs/internal/Subscriber.js:66:18)
     at /usr/src/app/node_modules/rxjs/internal/util/subscribeToPromise.js:7:24
     at processTicksAndRejections (internal/process/task_queues.js:89:5)

I'm using:

  • node v12.2.0
  • nestjs v6.1.0
  • nestjs-rate-limiter v1.0.1

As far as I can see, error happens in this catch block.

I would suggest either to return an observable after response has been sent or replace this with:

throw new HttpException({
                    statusCode: HttpStatus.TOO_MANY_REQUESTS,
                    error: 'Too Many Requests',
                    message: 'Rate limit exceeded.',
                }, 429);

Waiting on your response.

@RyanTheAllmighty
Copy link
Contributor

Sorry for the delayed response, somehow I missed this notification entirely.

I'll take a look at your suggestion and get back to you :)

@RyanTheAllmighty RyanTheAllmighty added bug Something isn't working help wanted Extra attention is needed labels Jul 12, 2019
@RyanTheAllmighty
Copy link
Contributor

So I've taken a look and tried both your suggestions, and I cannot get it to work. The exception is thrown and the logger picks it up, but the resulting http response is just a 500 error.

My experience with Typescript and NestJS is fairly new, so I'll keep looking into this. There's surely something I'm doing incorrectly, so I'll keep hacking away at this.

@edongashi
Copy link

From the docs:

Out of the box, this action is performed by a built-in global exception filter, which handles exceptions of type HttpException (and subclasses of it). When an exception is unrecognized (is neither HttpException nor a class that inherits from HttpException), the client receives the following default JSON response:

{
  "statusCode": 500,
  "message": "Internal server error"
}

I'm not sure though if this counts for interceptors after you eject to an http context.

@edongashi
Copy link

edongashi commented Jul 31, 2019

I think there's an actual internal server error here: Edit: That was my fault, I was playing with different config values.

[Nest] 7064   - 07/31/2019, 7:05 PM   [ExceptionsHandler] Invalid "type" option provided to RateLimiterInterceptor. Value was "undefined" +13834ms@my-app/server: Error: Invalid "type" option provided to RateLimiterInterceptor. Value was "undefined"
 at RateLimiterInterceptor.<anonymous> (node_modules\nestjs-rate-limiter\dist\rate-limiter.interceptor.js:90:27)

@d30jeff
Copy link

d30jeff commented Oct 26, 2019

I'm having the same issue.

Getting this error message when the limit has been reached.

    setTimeout(function () { throw err; }, 0);
                             ^

TypeError: You provided 'undefined' where a stream was expected. You can provide an Observable, Promise, Array, or Iterable.

@edongashi By any chance did you manage to find a way to solve this?

@edongashi
Copy link

@d30jeff I'm not sure. I think after configuring and testing in swagger it showed 429 properly. I have to test it again though because I haven't used this in production yet.

@maayoko
Copy link
Author

maayoko commented Oct 26, 2019

Sorry for late response. Don't remember every detail when I was debugging nestjs framework, however if you go check nestjs internal implementation for handling errors, its implemented in a way that every error that is instance of HttpException should return response with the message and status code that you provided, and if its not framework will return 500 and internal service error message (https://github.com/nestjs/nest/blob/master/packages/core/exceptions/base-exception-filter.ts#L29).

Since we are creating an instance of HttpException if (!(exception instanceof HttpException)) should evaluate to false, but somehow it evaluates to true.

Didn't have time to debug this issue any further so I handled the issue by creating my own exception filter (something similar to https://github.com/nestjs/nest/blob/master/sample/01-cats-app/src/common/filters/http-exception.filter.ts), but how you'll handle catching those errors depends on the requirements you'll have.

Hope this helps.

@RyanTheAllmighty
Copy link
Contributor

@maayoko yeah #10 I believe will fix this. I've noticed difference in between different nestjs versions on how this is handled, as initially throwing didn't work, but newest versions work as expected.

@macrosak
Copy link

@RyanTheAllmighty #10 fixed the issue for us as well, but the PR is closed right now. Could you reconsider merging it and releasing a new version?

onur-ozkan added a commit that referenced this issue Aug 31, 2020
@onur-ozkan
Copy link
Owner

Issue has been fixed with 011d418

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants