Lack of Error Handling + Unit Testing

yFaizuS commented 1 year ago

profile.js

```javascript ajaxRequest( 'api/account/profile', 'GET', JSON.stringify(data), function (response) { getProfile(response) }, function (jqXHR, textStatus, errorThrown) { // alert(jqXHR); }, headers ); } .... ajaxRequest( 'api/account/profile', 'PUT', JSON.stringify(data), function (response) { getProfileAjax() alert("Berhasil update data") }, function (jqXHR, textStatus, errorThrown) { // alert(jqXHR); if (jqXHR.status == 200) { alert("Berhasil update data") } else { alert("gagal") } }, headers ); ``` on the AJAX request the error handling part of the code is commented out / remarked. it is not handling error if an AJAX request error occurs

item.js

```javascript ajaxRequest( `api/dish/${idParams}`, 'GET', JSON.stringify(data), function (response) { console.log(response.description) $("#title").html(`Dish category - ${response.category}
${response.vegetarian ? 'vegetarian' : 'Not vegetarian'}`) $("#deskripsi").html(`${response.description}`) $("#harga").text(`${response.price} p`) }, function (jqXHR, textStatus, errorThrown) { // window.location.href = "order.html" }, headers ); ``` from the code snippet above, the error handling was supposed to redirect the user to the "order.html". by uncommenting the error handling when it redirects the user to "order.html" or adding a proper error response, so if any error occurs it would be handled accordingly

login.js

```javascript ajaxRequest( 'api/account/login', 'POST', JSON.stringify(data), function (response) { localStorage.setItem('token', response.token) window.location.href = 'index.html' alert("Login Success"); }, function (jqXHR, textStatus, errorThrown) { alert(jqXHR.responseJSON.message); }, headers ); ``` the code snippet above shows the error handling message but it can be changed into a much more readable and friendlier message to the user

order.js

```javascript ajaxRequest( `api/order/${id}`, 'GET', JSON.stringify(data), function (response) { getDetailOrder(response) }, function (jqXHR, textStatus, errorThrown) { }, headers ); ``` The code snippet above does not have an error handling in case of an error

LidiaIvanova commented 1 year ago

Why is this code bad? What principles does it violate? What can the existence of such code lead to? 0,8

yFaizuS commented 1 year ago

the lack of error handling can make debugging or troubleshooting difficult for developers, for what principle does it violates the (SRP)The Single Responsibility Principle states that a class should have only one reason to change. It suggests that a class or module should have a single responsibility or purpose. Neglecting error handling can result in a violation of SRP because error handling is an additional responsibility that is mixed with the primary functionality of the class. When error handling is not separated into its own module or handled by dedicated error-handling components, it can lead to bloated and less maintainable classes.

yFaizuS commented 1 year ago

Unit Testing

This unit test focus on the lack of the error handling I use Jest framework on testing the code

Profile.js

FIrst part of the Code Snippet:

const jquery = require('jquery'); const {JSDOM} = require('jsdom');

const dom = new JSDOM('<!DOCTYPE html>'); const {window} = dom;

global.window = window; global.document = window.document; global.$ = jquery(window);

global.localStorage = { store: {}, getItem(key) { return this.store[key]; }, setItem(key, value) { this.store[key] = value ; }, removeItem(key) { delete this.store[key]; }, };

const headers = { 'Content-Type': 'application/json', 'Authorization': Bearer ${localStorage.getItem('token')} };

const ajaxRequest = require('./profile.js');

describe('ajaxRequest error handling', () => { test('handles error in ajaxRequest', () => { // Mock the XMLHttpRequest object const mockXMLHttpRequest = jest.fn(); global.XMLHttpRequest = mockXMLHttpRequest;

// Mock the error callback function
const errorCallback = jest.fn();

// Perform the Ajax request
ajaxRequest(
  'api/account/profile',
  'GET',
  JSON.stringify({}),
  function (response) {
    // Success callback function
    getProfile(response);
  },
  errorCallback,
  headers
);

// Simulate an error in the Ajax request
const errorResponse = {
  status: 500,
  responseText: 'Internal Server Error',
};

// Expect the error callback to have been called
expect(errorCallback).toHaveBeenCalled();

}); });


In this example, we're testing the error handling scenario for the **`ajaxRequest`** function. the test asserts that an error response was called

yFaizuS commented 1 year ago

Item.js

const ajaxRequest = require('./item.js');

describe('ajaxRequest error handling', () => {
  test('handles error in ajaxRequest', () => {
    const mockXHR = {
      open: jest.fn(),
      setRequestHeader: jest.fn(),
      send: jest.fn(),
      readyState: 4,
      status: 500,
      responseText: 'Internal server error',
    };
    global.XMLHttpRequest = jest.fn(() => mockXHR);

    const successCallback = jest.fn();
    const errorCallback = jest.fn();

    const idParams = '3fa85f64-5717-4562-b3fc-2c963f66afa6';
    ajaxRequest(
      `api/dish/${idParams}`,
      'GET',
      JSON.stringify({}),
      successCallback,
      errorCallback,
      {}
    );

    expect(mockXHR.open).toHaveBeenCalled();
    expect(mockXHR.setRequestHeader).toHaveBeenCalled();
    expect(mockXHR.send).toHaveBeenCalled();

    mockXHR.onreadystatechange(); // Simulate the readyState change

    expect(successCallback).not.toHaveBeenCalled();
    expect(errorCallback).toHaveBeenCalled();
    expect(errorCallback).toHaveBeenCalledWith(mockXHR, mockXHR.statusText, mockXHR.responseText);
  });
});

We're testing if the ajaxRequest function handles errors correctly. the test simulates an error response from the server by mocking the XMLHttpRequest . By testing the error handling of the function we can be sure that it behaves as expected when encountering errors

yFaizuS commented 1 year ago

login.js

const jsdomGlobal = require('jsdom-global');
const cleanup = jsdomGlobal();

const ajaxRequest = require('./login.js');

describe('ajaxRequest error handling', () => {
  let mockXHR;

  beforeEach(() => {
    mockXHR = {
        open: jest.fn(),
        setRequestHeader: jest.fn(),
        send: jest.fn(),
        readyState: 4,
        status: 0,
        responseText:'',
        onreadystatechange: null,
    }
    global.XMLHttpRequest = jest.fn(() => mockXHR);
  });

  afterEach(() => {
    jest.clearAllMocks();
  });

  afterAll(() => {
    cleanup();
  });

  test('handles error in ajaxRequest', () => {
    const errorCallback = jest.fn();

    ajaxRequest(
      'api/account/login',
      'POST',
      JSON.stringify({}),
      function (response) {
        localStorage.setItem('token', response.token);
        window.location.href = 'index.html';
        console.log('Login Success');
      },
      errorCallback, // Pass the errorCallback function directly
      {}
    );

    const errorResponse = {
      status: 500,
      responseJSON: {
        message: 'Internal Server Error',
      },
    };

    mockXHR.status = errorResponse.status;
    mockXHR.responseText = JSON.stringify(errorResponse.responseJSON);
    mockXHR.onreadystatechange();

    expect(errorCallback).toHaveBeenCalled();
  });
});

In this test, we ensure that ajaxRequest function handles error responses from the server properly. it verifies that when an error response is received, the provided errorCallback function is called, the test asserts that the errorCallback function has been called properly

yFaizuS commented 1 year ago

order.js

const jsdomGlobal = require('jsdom-global');
const cleanup = jsdomGlobal();

const ajaxRequest = require('./order.js');

describe('ajaxRequest error handling', () => {
  let mockXHR;

  beforeEach(() => {
    mockXHR = {
      open: jest.fn(),
      setRequestHeader: jest.fn(),
      send: jest.fn(),
      readyState: 4,
      status: 0,
      responseText: '',
      addEventListener: jest.fn(),
    };
    global.XMLHttpRequest = jest.fn(() => mockXHR);
  });

  afterEach(() => {
    jest.clearAllMocks();
  });

  afterAll(() => {
    cleanup();
  });

  test('handles error in ajaxRequest', (done) => {
    jest.setTimeout(1000);
    const errorCallback = jest.fn();
    const successCallback = jest.fn();

    const id = '3fa85f64-5717-4562-b3fc-2c963f66afa6';
    ajaxRequest(
      `api/order/${id}`,
      'GET',
      JSON.stringify({}),
      successCallback,
      errorCallback,
      {}
    );

    expect(mockXHR.open).toHaveBeenCalled();
    expect(mockXHR.setRequestHeader).toHaveBeenCalled();
    expect(mockXHR.send).toHaveBeenCalled();

    mockXHR.status = 500;
    mockXHR.addEventListener.mock.calls[0][1](); // Simulate the readyState change

    setTimeout(() => {
      expect(successCallback).not.toHaveBeenCalled();
      expect(errorCallback).toHaveBeenCalled();
      expect(errorCallback).toHaveBeenCalledWith(
        mockXHR,
        mockXHR.statusText,
        mockXHR.responseText
      );
      done();
    }, 0);
  });
});

the test checking the error handling functionality of the ajaxRequest function. it asserts that when an error occurs during AJAX request, the appropriate error callback is called

yFaizuS / Delivery-International