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

Can not catch error when start listen #69

Open
sounisi5011 opened this issue Jun 2, 2020 · 0 comments
Open

Can not catch error when start listen #69

sounisi5011 opened this issue Jun 2, 2020 · 0 comments

Comments

@sounisi5011
Copy link

The JSDoc of the Git.listen() method states that "the function to call when server is started or error has occured" in the description of the callback argument.

node-git-server/lib/git.js

Lines 429 to 441 in 5279f64

/**
* starts a git server on the given port
* @method listen
* @memberof Git
* @param {Number} port - the port to start the server on
* @param {Object=} options - the options to add extended functionality to the server
* @param {String=} options.type - this is either https or http (the default is http)
* @param {Buffer|String=} options.key - the key file for the https server
* @param {Buffer|String=} options.cert - the cert file for the https server
* @param {Function} callback - the function to call when server is started or error has occured
* @return {Git} - the Git instance, useful for chaining
*/
listen(port, options, callback) {

However, in reality, if the server fails to start with an error, the callback is not called.
In addition, this error cannot be catched even with try...catch statement.

The Server object is a Node.js EventEmitter. As with all EventEmitter's, most errors are passed to the 'error' event. If no handler is registered for the 'error' event, those bubble up to be thrown. try/catch does not work because when listen is attempted, any errors that occur are caught and emitted on the 'error' event using process.nextTick() -- that is, by the time the error is actually reported, the try/catch block has already exited. As a fallback, you can register an 'uncaughtException' handler on process as a catch all for any unhandled errors that occur on any EventEmitter object, but it's best to simply set the 'error' callback on the server object.

Originally posted by @jasnell in expressjs/express#2856 (comment)

The best way to resolve this problem is to register the error event on the http.Server object to get the error.
Currently (version 0.6.1) we can get the error in the following ways:

const Server = require('node-git-server');

const repos = new Server('path/to/tmp', {
    autoCreate: true
});
const port = process.env.PORT || 7005;

// ...

repos.listen(port, () => {
    console.log(`Success!`)
});
repos.server.on('error', error => {
    console.error(`Error: ${error}`);
});

However, this solution requires the use of the server property that are not in the documentation.
In addition, this method is not intuitive. The user cannot catch the error in the callback, nor in the try...catch statement. This solution requires knowledge of Node.js built-in http/https modules.

The ideal solution is a change that allows the function in the callback argument to catch the error.
To do this, this project needs to make the following changes to the Git.listen() method:

    listen(port, options, callback) {
        const self = this;
        if(typeof options == 'function' || !options) {
          callback = options;
          options = { type: 'http' };
        }
        const createServer = options.type == 'http' ? http.createServer : https.createServer.bind(this, options);

        this.server = createServer(function(req, res) {
            self.handle(req, res);
        });

+       this.server.on('error', callback);
+
        this.server.listen(port, callback);
        return this;
    }
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

1 participant