refactor: migrate errors server controller to typescript

[tshuli]

Part of #144

[karrui]

Some changes requested, with the main one being to change it to a pure utility file.

Please try to think of ways to improve the code too when you are refactoring instead of plainly converting JS to TS code! The main point we are doing this is to reduce tech debt/legacy code after all :D

[karrui]

nit: prefer explicit destructure of used imports:

// If only one is being used.
import isEmpty from 'lodash/isEmpty'
// If multiple lodash functions are being used in this file
import { isEmpty, somethingElse } from 'lodash'

[karrui]

nit: Remove types from JSDocs, it's already typed in the function signature

[karrui]

At first glance, I thought this is only used in one place, but seems to also be used in tests. Add a comment stating this is also exported for tests? Though I don't actually think it's necessary, we can also just hardcode this string in the tests.

If you still want to export it, constants should be named in UPPER_SNAKE_CASE for consistency.

Suggested change

	export const defaultErrorMessage =
	export const DEFAULT_ERROR_MSG =

[karrui]

Not a fan of this function, I think it is too hackish instead of fixing the base issue.

In fact, in the entire schema, there is only one instance of a schema with a unique parameter, i.e UserModel, and that case is already handled by it's own post-validation hook. Meaning we do not even need this function and this function can be deleted

[karrui]

We can also keep it in if you want, but I'm not a fan of this weird error message interpolation.

[karrui]

Please update all functions that do not need the this parameter (which is all of the functions in this file) into arrow functions.

See further reading for differences between arrow functions and regular functions

[karrui]

Should also move and rename this file to the utility function it is, probably into utils/handleMongoError.ts

Read more about utility vs service functions/classes here, and read more about the differences between service and controllers layers (this is more Java-like but the point still stands) to see why the functions in this file does not belong in the controller layer. Do hit me (or Google) up for a more detailed explanation if you still have any queries

[karrui]

This whole function seems like it needs a good refactor, maybe we should try to clean up the function?

I don't think we should create the IMongoError interface when we already have the typings of the possible errors:

import { MongoError } from 'mongodb'
import { Error as MongooseError } from 'mongoose'

...
export const getMongoErrorMessage = (
  err?: MongoError | MongooseError | string,
): string => { ... }

[karrui]

Suggested refactor:

export const getMongoErrorMessage = (
  err?: MongoError | MongooseError | string,
): string => {
  if (!err) {
    return ''
  }

  // Handle base Mongo engine errors.
  if (err instanceof MongoError) {
    switch (err.code) {
      case 10334: // BSONObj size invalid error
        return 'Your form is too large to be supported by the system.'
      default:
        return defaultErrorMessage
    }
  }

  // Handle mongoose errors.
  if (err instanceof MongooseError.ValidationError) {
    // Join all error messages into a single message if available.
    const joinedMessage = Object.values(err.errors)
      .map((err) => err.message)
      .join(', ')

    return joinedMessage ?? err.message ?? defaultErrorMessage
  }

  if (err instanceof MongooseError) {
    return err.message ?? defaultErrorMessage
  }

  return err ?? defaultErrorMessage
}

[karrui]

Unneeded, as explained above

[karrui]

Remember to update tests

[karrui]

Any updates on this PR?

[tshuli]

Closing first to deprioritise, will pick up again after helium