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

[NEW TEST] Testing for interceptor failure #1503

Open
1 task done
joebowbeer opened this issue Dec 12, 2021 · 3 comments
Open
1 task done

[NEW TEST] Testing for interceptor failure #1503

joebowbeer opened this issue Dec 12, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@joebowbeer
Copy link

joebowbeer commented Dec 12, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Feature Test To Be Requested

I was using your CatInterceptor test as a template for a request interceptor test.

It was not clear how to test for an expected failure. Below is sketch of the test I created:

class FailingInterceptor implements NestInterceptor {
  intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
    throw new Error();
  }
}

const interceptor = new FailingInterceptor();

// create the mock CallHandler for the interceptor
const next = {
  handle: () => EMPTY,
};

it('should fail', () => {
  const ctxMock = createMock<ExecutionContext>({
    switchToHttp: () => ({
      getRequest: () => ({
        headers: {},
      }),
    }),
  });
  expect(() =>
    interceptor
      .intercept(ctxMock, next)
      .subscribe({ complete: () => fail() }),
  ).toThrowError(Error);
  expect(fnAfterFailure).not.toHaveBeenCalled();
});

I also discovered that failures in the Observable subscriber would result in test timeouts. That is, the test would fail but it would also timeout because done was never called.

To be resilient in the event of unexpected failure, I think the cat interceptor test should be modified:

    it('should successfully return', (done) => {
      interceptor.intercept({} as any, next).subscribe({
        next: (value) => {
          try {
            expect(value).toEqual({ data: returnCat });
          } catch(error) {
            fail(error);
          }
        },
        error: (error) => {
          fail(error);
        },
        complete: () => {
          done();
        },
      });
    });
@joebowbeer joebowbeer added the enhancement New feature or request label Dec 12, 2021
@micalevisk
Copy link
Contributor

micalevisk commented Dec 28, 2021

regarding testing interceptors, given a TimeoutInterceptor that looks like this:

import {
  Injectable,
  NestInterceptor,
  ExecutionContext,
  CallHandler,
  RequestTimeoutException,
} from '@nestjs/common';
import ms from 'ms';
import { Observable, throwError, TimeoutError } from 'rxjs';
import { catchError, timeout } from 'rxjs/operators';

export class TimeoutInterceptor implements NestInterceptor {
  static TIMEOUT_RESPONSE_TIME_MS = ms('5s');

  intercept(_ctx: ExecutionContext, next: CallHandler): Observable<unknown> {
    return next.handle().pipe(
      timeout(TimeoutInterceptor.TIMEOUT_RESPONSE_TIME_MS),
      catchError((err) => {
        if (err instanceof TimeoutError) {
          return throwError(new RequestTimeoutException());
        }
        return throwError(err);
      }),
    );
  }
}

I've made an unit test for it, timeout.interceptor.spec.ts:

import { createMock } from '@golevelup/ts-jest';
import { CallHandler, ExecutionContext, RequestTimeoutException } from '@nestjs/common';
import { throwError } from 'rxjs';
import { marbles } from 'rxjs-marbles/jest';

import { TimeoutInterceptor } from '@/api/interceptors/timeout.interceptor';

describe('TimeoutInterceptor', () => {
  const TIMEOUT = TimeoutInterceptor.TIMEOUT_RESPONSE_TIME_MS;
  let timeoutInterceptor: TimeoutInterceptor;

  beforeEach(() => {
    timeoutInterceptor = new TimeoutInterceptor();
  });

  it(`should return the data, when the request handler take less than ${TIMEOUT}ms to execute`,
    marbles((m) => {
      const ctx = createMock<ExecutionContext>();
      const expectedData = {};
      const next: CallHandler = {
        handle: () => m.cold('(e|)', { e: expectedData }),
      };

      const handlerData$ = timeoutInterceptor.intercept(ctx, next);

      /** Marble emiting the value `expectedData` and complete. */
      const expected$ = m.cold('(e|)', { e: expectedData });
      m.expect(handlerData$).toBeObservable(expected$);
    }),
  );

  it(`should throw RequestTimeoutException (HTTP 408) error, when the request handler take ${TIMEOUT}ms to execute`,
    marbles((m) => {
      const ctx = createMock<ExecutionContext>();
      const next = {
        handle: () => m.cold(`${TIMEOUT}ms |`),
      };

      const handlerData$ = timeoutInterceptor.intercept(ctx, next);

      /** Marble emiting an error after `TIMEOUT` ms. */
      const expected$ = m.cold(`${TIMEOUT}ms #`, undefined, new RequestTimeoutException());
      m.expect(handlerData$).toBeObservable(expected$);
    }),
  );

  it(`should forward the error thrown`,
    marbles((m) => {
      const ctx = createMock<ExecutionContext>();
      const error = new Error('something');
      const next = {
        handle: () => throwError(error),
      };

      const handlerData$ = timeoutInterceptor.intercept(ctx, next);

      /** Marble emiting an error after `TIMEOUT`ms. */
      const expected$ = m.cold(`#`, undefined, error);
      m.expect(handlerData$).toBeObservable(expected$);
    }),
  );

  it(`should throw RequestTimeoutException (HTTP 408) error, when the request handler take ${2 * TIMEOUT}ms to run`,
    marbles((m) => {
      const ctx = createMock<ExecutionContext>();
      const next = {
        handle: () => m.cold(`${2 * TIMEOUT}ms |`),
      };

      const handlerData$ = timeoutInterceptor.intercept(ctx, next);

      /** Marble emiting the error after `TIMEOUT`ms. */
      const expected$ = m.cold(`${TIMEOUT}ms #`, undefined, new RequestTimeoutException());
      m.expect(handlerData$).toBeObservable(expected$);
    }),
  );
});

@jmcdo29
Copy link
Owner

jmcdo29 commented Jan 8, 2022

By the way @micalevisk if you want to put this into a sample app and add a PR that would be awesome! If not, I'm sure I'll get around to it eventually :)

@micalevisk
Copy link
Contributor

@jmcdo29 I was thinking on adding a very basic sample that uses that TimeoutInterceptor just to demonstrate how we can write an unit test for the timeout operator of rxjs. But I don't think this will be that useful as it doesn't do anything fancy for the nestjs side, it just uses the API of rxjs-marbles lib to play with rxjs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants