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

A proposal about error handling on a user defined function in server.js file #538

Closed
efkan opened this issue Nov 6, 2017 · 2 comments
Closed

Comments

@efkan
Copy link
Contributor

efkan commented Nov 6, 2017

Current behaviour

In server.js file, generateId errors are handled as the below:

  this.generateId(req, function (err, id) {
    if (err) {
      sendErrorMessage(req, req.res, Server.errors.BAD_REQUEST);
      return;
    }
    ...
  }

Because of generateId method can be overwritten by the user, it should be throw the error what the user sent.

My proposal:

  this.generateId(req, function (err, id) {
    if (err) {
      sendErrorMessage(req, req.res, err);
      return;
    }
    ...
  }

I could not decide this is the correct way to handle errors or not. Because, the user function might throws an error that designed by the user.

However, the error turns into a BAD_REQUEST error in the end.

On the other hand this is a part of socket.io and socket.io-client-java modules do not handle the error of BAD_REQUEST on any method (if I'm right).

For instance I throw different error codes on generateId method on different cases. Then I handle this error on client side (like invalidate_auth_token).

I use a workaround for this for now. I send error codes as socketId then I check the socketId value in a socket middleware. If socketId value is one of the error codes, then disconnect the connection and send the error code to the client.

@efkan efkan changed the title A question about error handling method in server.js file A proposal about error handling on a user defined function in server.js file Nov 6, 2017
@efkan
Copy link
Contributor Author

efkan commented Mar 7, 2018

Note: there is another breaking change problem with generateId method.
#535 (comment)

If the problem cannot be solved this issue must be closed.

@darrachequesne
Copy link
Member

Closed due to inactivity, please reopen if needed.

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

No branches or pull requests

2 participants