haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.02k stars 662 forks source link

Error in init_http with new 2.8.7 #1520

Closed ricardopolo closed 8 years ago

ricardopolo commented 8 years ago

This code used to work

    this.register_hook('init_http', 'load_certificate');
  //Loads the certificates to TLS at startup, if not stop Haraka
  exports.load_certificate = function (next, server) {
    async.series(
      [
        function (callback){
          fs.readFile('file', function (err, data) {
            if(err || !data) return callback(err);
            callback();
          });
        },
      ],function (err){
        if(err){
          server.logerror('Error');
          server.sendToMaster('gracefulShutdown');
        }
        else {
          return next();
        }
      }
    );
  };

Now in Haraka 2.8.7 when the plugin starts with receive this exception

TypeError: Server.http.wss.unref is not a function
baudehlo commented 8 years ago

Need to find where wss stores the actual socket. Or find a better way to shut it down cleanly.

On Jun 22, 2016, at 6:51 PM, Ricardo Polo notifications@github.com wrote:

This code used to work

this.register_hook('init_http', 'load_certificate');

//Loads the certificates to TLS at startup, if not stop Haraka exports.load_certificate = function (next, server) { async.series( [ function (callback){ fs.readFile('file', function (err, data) { if(err || !data) return callback(err); callback(); }); }, ],function (err){ if(err){ server.logerror('Error'); server.sendToMaster('gracefulShutdown'); } else { return next(); } } ); }; Now in Haraka 2.8.7 when the plugin starts with receive this exception

TypeError: Server.http.wss.unref is not a function — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

ricardopolo commented 8 years ago

@baudehlo meanwhile is possible to have express without WSS? we just need a basic website, whitout wecksockets.

msimerson commented 8 years ago

Need to find where wss stores the actual socket.

WSS is an upgrade of an existing HTTP connection. See here, where the WSS "socket" is created by passing in an existing HTTP server object.

The problem is that that wss.unref() that naively / blindly attempting to terminate the wss handle, even if it doesn't exist.

msimerson commented 8 years ago

@ricardopolo, just delete this line and it'll likely work fine:

https://github.com/haraka/Haraka/blob/master/server.js#L554

ricardopolo commented 8 years ago

@msimerson Nice!!

Let's tag 2.8.7.1! :shipit:

baudehlo commented 8 years ago

Probably not the way the code is structured. You'll need to find a patch that fixes it (or I'll look into it in a bit).

On Jun 22, 2016, at 7:05 PM, Ricardo Polo notifications@github.com wrote:

@baudehlo meanwhile is possible to have express without WSS? we just need a basic website, whitout wecksockets.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

baudehlo commented 8 years ago

I'd like to see the outbound dkim change reviewed, then we can push 2.8.8 with that fix.

On Jun 22, 2016, at 7:25 PM, Ricardo Polo notifications@github.com wrote:

@msimerson Nice!!

Let's tag 2.8.7.1! 💃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

msimerson commented 8 years ago

I'd like to see the outbound dkim change reviewed

Why buck our 2.8 trend of reviewing and testing code via merge & release?

ricardopolo commented 8 years ago

Have you ever thought about automatically release a new version with every commit to the master using a Continuos Integration server? I am thinking about the Github Flow style, every build to master could be consider deployable and auto release a new minor version. And main releases done after extensive testing and code.

msimerson commented 8 years ago

That can already be done. If you want to install Haraka from the github.com repo, npm is happy to oblige.

baudehlo commented 8 years ago

@msimerson don't be passive aggressive please - you're our most strict and best release manager, and hugely appreciated for it, but you haven't been around much lately, and so things have slipped through the cracks.

msimerson commented 8 years ago

don't be passive aggressive please

Sorry, I was actually feeling rather humorous but that didn't come through. I was also half serious: releasing does get the code "out there" and we can always follow up with another quick bug-fix release that addresses issues that slip through.

I don't see the stuff slipping through as an issue with the release process as much as it's an issue with the testability and test coverage of the code. That's really the only insurance that features work as designed and will continue to work as changes are made.

baudehlo commented 8 years ago

Agreed 100%. I would like to have a coverage blitz in the next few months.

On Jun 22, 2016, at 10:21 PM, Matt Simerson notifications@github.com wrote:

don't be passive aggressive please

Sorry, I was actually feeling rather humorous but that didn't come through. I was also half serious: releasing does get the code "out there" and we can always follow up with another quick bug-fix release that addresses issues that slip through.

I don't see the stuff slipping through as an issue with the release process as much as it's an issue with the testability and test coverage of the code. That's really the only insurance that features work as designed and will continue to work as changes are made.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

msimerson commented 8 years ago

I'm mostly offline for the summer due to another project but part of my long term / work-in-progress strategy to get Haraka coverage into shape is:

  1. move plugins to their own repos. It's really easy to update & release small packages with near-complete test coverage like haraka-tld (95%) and haraka-config (76%). When bad stuff happens, it doesn't take long to make sure that issue never happens again. We've recently made a lot of progress towards plugins in their own repos.
  2. refactor the core for maintainability and testability. We've got a solid core that works really well. It's just too easy to release a crasher while working in there.
  3. refactor portions of Haraka into separate well-tested modules (ala haraka-config).