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

No Longer works with Hapi 9.3.1 #12

Open
dp1140a opened this issue Sep 10, 2015 · 11 comments
Open

No Longer works with Hapi 9.3.1 #12

dp1140a opened this issue Sep 10, 2015 · 11 comments

Comments

@dp1140a
Copy link

dp1140a commented Sep 10, 2015

Hapi 9.3.1 requires that the Inert plugin be manually included for static content. This breaks grunt-hapi which now gives the error:

Fatal error: Hapi ["/Users/dpatton/Documents/Git/HDFS-Viz/server/server.js"] - Error: Missing required start callback function

Here is the relevant section of my server file

const Hapi = require('hapi'); //http://hapijs.com/api
const inert = require('inert');

const server = new Hapi.Server();
server.connection({
host: config.server.host,
address: config.server.host,
port: config.server.port
});

server.register(inert, function (err) {});

server.route({
method: 'GET',
path: '/{param*}',
handler: {
directory: {
path: process.cwd() + '/app',
index: true
}
}
});

/** Other routes ... **/

/***********************
Server Start
***********************/
server.start(function (err) {
if (err) {
log.error(err);
}
log.info('Server running at ' + server.info.uri);
});
module.exports = server;

@athieriot
Copy link
Owner

Hey thank you very much for the report.

I am going to take a look at what can be done.
In the meantime, if you fancy to look at it, I would happily be available to review a Pull Request as well.

Up to you.

Aurélien

@dp1140a dp1140a changed the title No Longer works with Hapi 9.X No Longer works with Hapi 9.3.1 Sep 10, 2015
@athieriot
Copy link
Owner

All right, I just publish a new version: 0.10.0

Would you mind trying with your configuration to see is act as usual?

Please note as well that grunt-hapi accept a property named bases that should register as many path of static files as you want for you.

@athieriot
Copy link
Owner

Hi there,

I was wondering if you had time to take a look at that release?
Just wanted to be sure it's working

Thanks

@dp1140a
Copy link
Author

dp1140a commented Sep 28, 2015

The bad news is it still doesnt work. The good news is that it gives a new error:

Fatal error: Hapi ["/Users/dpatton/Documents/Git/HDFS-Viz/server/server.js"] - Error: Cannot start server while it is in initializing state

I also tried this with Hapi 10.3.1

@athieriot
Copy link
Owner

The only thing I can think of right now is that Hapi might get started twice.

Please note that grunt-hapi will start the server on his own. It is somehow necessary to run tests with grunt-contrib-watch correctly.

Starting the server can be put in a start script (and used only in Production), letting Grunt manage it for Dev and Testing. This was how the grunt-express plugin was working.

If it's not ideal, we can think about something else but in the meantime, would you be kind enough to tell me if it works without calling start yourself?

@dp1140a
Copy link
Author

dp1140a commented Sep 29, 2015

If I start via node server/server.js it starts fine.

@athieriot
Copy link
Owner

I am going to take a look.

Would you mind to paste your Grunt configuration (The grunt-hapi part would suffice) please?

My point was, if the server script you give to grunt-hapi is trying to start the server (As it is suggested by the content of the PR), grunt-hapi will try to do it too (Cause it needs to control the life cycle for the watch process to work).

And I guess that you can't "start" an hapi server twice.

@dp1140a
Copy link
Author

dp1140a commented Oct 7, 2015

Heres the whole Gruntfile:

// Generated on 2014-12-16 using
// generator-webapp 0.5.1
'use strict';

// # Globbing
// for performance reasons we're only matching one level down:
// 'test/spec/{,/}.js'
// If you want to recursively match all subfolders, use:
// 'test/spec/*/.js'

module.exports = function(grunt) {

// Time how long tasks take. Can help when optimizing build times
require('time-grunt')(grunt);

// Load grunt tasks automatically
require('load-grunt-tasks')(grunt);

// Configurable paths
var config = {
    app: 'app',
    bower: 'app/bower_components',
    server: 'server',
    dist: 'dist',
    distApp: 'dist/app',
    distServer: 'dist/server'
};

grunt.verbose;
// Define the configuration for all the tasks
grunt.initConfig({

    // Project settings
    config: config,

    // Watches files for changes and runs tasks based on the changed files
    watch: {
        bower: {
            files: ['bower.json'],
            tasks: ['wiredep']
        },
        js: {
            files: ['<%= config.app %>/{,*/}*.js'],
            tasks: ['jshint'],
            options: {
                livereload: true
            }
        },
        jstest: {
            files: ['test/spec/{,*/}*.js'],
            tasks: ['test:watch']
        },
        gruntfile: {
            files: ['Gruntfile.js'],
            options: {
                livereload: true
            }
        },
        styles: {
            files: ['<%= config.app %>/styles/{,*/}*.css'],
            tasks: ['newer:copy:styles', 'autoprefixer']
        },
        hapi: {
            files: ['<%= config.server %>/**/*.js'],
            tasks: ['hapi'],
            options: {
                spawn: false // Newer versions of grunt-contrib-watch might require this parameter.
            }
        },
        livereload: {
            options: {
                livereload: '<%= connect.options.livereload %>'
            },
            files: [
                '<%= config.server %>/{,*/}*.js'
            ]
        }
    },

    hapi: {
        custom_options: {
            options: {
                docs: true,
                server: require('path').resolve('server/server.js'),
                port: '9001',
                bases: {
                    '/app': require('path').resolve('./app/')
                }
            }
        }
    },

    // The actual grunt server settings
    connect: {
        options: {
            port: 9000,
            open: true,
            livereload: 35729,
            // Change this to '0.0.0.0' to access the server from outside
            hostname: 'localhost'
        },
        livereload: {
            options: {
                middleware: function(connect) {
                    return [
                        connect.static('.tmp'),
                        connect().use('/bower_components', connect.static('./bower_components')),
                        connect.static(config.app)
                    ];
                }
            }
        },
        test: {
            options: {
                open: false,
                port: 9001,
                middleware: function(connect) {
                    return [
                        connect.static('.tmp'),
                        connect.static('test'),
                        connect().use('/bower_components', connect.static('./bower_components')),
                        connect.static(config.app)
                    ];
                }
            }
        },
        dist: {
            options: {
                base: '<%= config.dist %>',
                livereload: false
            }
        }
    },

    // Empties folders to start fresh
    clean: {
        dist: {
            files: [{
                dot: true,
                src: [
                    '.tmp',
                    '<%= config.dist %>/**',
                    '!<%= config.dist %>/.git*',
                    '<%= config.dist %>'
                ]
            }]
        },
        server: '.tmp'
    },

    // Make sure code styles are up to par and there are no obvious mistakes
    jshint: {
        options: {
            jshintrc: '.jshintrc',
            reporter: require('jshint-stylish'),
            node: true,
            ignores: ['<%= config.app %>/scripts/old/{,*/}*.js'],
            globals: {
                'd3': true,
                'HADOOPOPTS': true
            }
        },
        all: [
            '<%= config.server %>/server.js',
            '<%= config.app %>/scripts/{,*/}*.js'
        ]
    },

    // Mocha testing framework configuration options
    mocha: {
        all: {
            options: {
                run: true,
                urls: ['http://<%= connect.test.options.hostname %>:<%= connect.test.options.port %>/index.html']
            }
        }
    },

    // Add vendor prefixed styles
    autoprefixer: {
        options: {
            browsers: ['> 1%', 'last 2 versions', 'Firefox ESR', 'Opera 12.1']
        },
        dist: {
            files: [{
                expand: true,
                cwd: '.tmp/styles/',
                src: '{,*/}*.css',
                dest: '.tmp/styles/'
            }]
        }
    },

    // Automatically inject Bower components into the HTML file
    wiredep: {
        app: {
            ignorePath: /^\/|\.\.\//,
            src: ['<%= config.app %>/index.html'],
            exclude: ['bower_components/bootstrap/dist/js/bootstrap.js']
        }
    },

    // Renames files for browser caching purposes
    rev: {
        dist: {
            files: {
                src: [
                    '<%= config.dist %>/scripts/{,*/}*.js',
                    '<%= config.dist %>/styles/{,*/}*.css',
                    '<%= config.dist %>/images/{,*/}*.*',
                    '<%= config.dist %>/styles/fonts/{,*/}*.*',
                    '<%= config.dist %>/*.{ico,png}'
                ]
            }
        }
    },

    // Reads HTML for usemin blocks to enable smart builds that automatically
    // concat, minify and revision files. Creates configurations in memory so
    // additional tasks can operate on them
    useminPrepare: {
        options: {
            dest: '<%= config.distApp %>'
        },
        html: ['<%= config.app %>/index.html', '<%= config.app %>/{,*/}*.html']
    },

    // Performs rewrites based on rev and the useminPrepare configuration
    usemin: {
        options: {
            assetsDirs: [
                '<%= config.distApp %>',
                '<%= config.distApp %>/images',
                '<%= config.distApp %>/styles'
            ]
        },
        html: ['<%= config.distApp %>/{,*/}*.html'],
        css: ['<%= config.distApp %>/styles/{,*/}*.css']
    },

    // The following *-min tasks produce minified files in the dist folder
    imagemin: {
        dist: {
            files: [{
                expand: true,
                cwd: '<%= config.app %>/images',
                src: '{,*/}*.{gif,jpeg,jpg,png}',
                dest: '<%= config.distApp %>/images'
            }]
        }
    },

    svgmin: {
        dist: {
            files: [{
                expand: true,
                cwd: '<%= config.app %>/images',
                src: '{,*/}*.svg',
                dest: '<%= config.distApp %>/images'
            }]
        }
    },

    htmlmin: {
        dist: {
            options: {
                removeComments: true,
                collapseBooleanAttributes: true,
                collapseWhitespace: true,
                conservativeCollapse: true,
                removeAttributeQuotes: true,
                removeCommentsFromCDATA: true,
                removeEmptyAttributes: true,
                removeOptionalTags: true,
                removeRedundantAttributes: true,
                useShortDoctype: true
            },
            files: [{
                expand: true,
                cwd: '<%= config.dist %>',
                src: '**/*.html',
                dest: '<%= config.dist %>'
            }]
        }
    },
    uglify: {
        options: {
            report: 'min'
        },
        dist: {
            files: {
                '<%= config.distApp %>/scripts/hdfsWorker.js': [
                    '<%= config.app %>/scripts/hdfsWorker.js'
                ]
            }
        }
    },

    // By default, your `index.html`'s <!-- Usemin block --> will take care
    // of minification. These next options are pre-configured if you do not
    // wish to use the Usemin blocks.
    // cssmin: {
    //   dist: {
    //     files: {
    //       '<%= config.dist %>/styles/main.css': [
    //         '.tmp/styles/{,*/}*.css',
    //         '<%= config.app %>/styles/{,*/}*.css'
    //       ]
    //     }
    //   }
    // },
    // uglify: {
    //   dist: {
    //     files: {
    //       '<%= config.dist %>/scripts/scripts.js': [
    //         '<%= config.dist %>/scripts/scripts.js'
    //       ]
    //     }
    //   }
    // },
    // concat: {
    //   dist: {}
    // },

    // Copies remaining files to places other tasks can use
    copy: {
        dist: {
            files: [{
                expand: true,
                dot: true,
                cwd: '<%= config.app %>',
                dest: '<%= config.distApp %>',
                src: [
                    '*.{ico,png,gif,txt}',
                    'images/{,*/}*.webp',
                    '{,*/}*.html',
                    'styles/fonts/{,*/}*.*'
                ]
            }, {
                src: '<%= config.server %>/**',
                dest: '<%= config.dist %>/'
            }, {
                src: 'package.json',
                dest: '<%= config.dist %>/'
            }, {
                //for bootstrap fonts
                expand: true,
                dot: true,
                cwd: '<%= config.bower %>/bootstrap/dist',
                src: ['fonts/*.*'],
                dest: '<%= config.distApp %>'
            }, {
                //for font-awesome
                expand: true,
                dot: true,
                cwd: '<%= config.bower %>/font-awesome',
                src: ['fonts/*.*'],
                dest: '<%= config.distApp %>'
            }]
        },
        styles: {
            expand: true,
            dot: true,
            cwd: '<%= config.app %>/styles',
            dest: '.tmp/styles/',
            src: '{,*/}*.css'
        }
    },

    // Run some tasks in parallel to speed up build process
    concurrent: {
        server: [
            'copy:styles'
        ],
        test: [
            'copy:styles'
        ],
        dist: [
            'copy:styles',
            'imagemin',
            'svgmin'
        ]
    }
});


grunt.registerTask('serve', 'start the server and preview your app, --allow-remote for remote access', function(target) {
    if (grunt.option('allow-remote')) {
        grunt.config.set('connect.options.hostname', '0.0.0.0');
    }
    if (target === 'dist') {
        return grunt.task.run(['build', 'connect:dist:keepalive']);
    }

    grunt.task.run([
        'clean:server',
        'wiredep',
        'concurrent:server',
        'autoprefixer',
        'connect:livereload',
        'hapi',
        'watch'
    ]);
});

grunt.registerTask('startHapi', function(target) {
    grunt.task.run([
        'clean:server',
        'wiredep',
        'concurrent:server',
        'autoprefixer',
        'hapi',
        'watch'
    ]);
});

grunt.registerTask('test', function(target) {
    if (target !== 'watch') {
        grunt.task.run([
            'clean:server',
            'concurrent:test',
            'autoprefixer'
        ]);
    }

    grunt.task.run([
        'connect:test',
        'mocha'
    ]);
});

grunt.registerTask('build', [
    'clean:dist',
    'wiredep',
    'useminPrepare',
    'concurrent:dist',
    'autoprefixer',
    'concat',
    'cssmin',
    'uglify',
    'copy:dist',
    'rev',
    'usemin',
    'htmlmin'
]);

grunt.registerTask('default', [
    'newer:jshint',
    'test',
    'build'
]);

};

@athieriot
Copy link
Owner

Thank you ! That is very helpful.

I can't help but to try to push my idea to the end though.
Please accept my apology for being a bit heavy.

The script you give as a server entry to grunt-hapi (require('path').resolve('server/server.js')) is definitively starting the server on its own. And that will conflict with grunt-hapi's own start statement. Leading to the error you are experiencing now.

I would be really curious to see the result of removing that part from your server.js file:

server.start(function (err) {
   if (err) {
      log.error(err);
   }
   log.info('Server running at ' + server.info.uri);
});

And moved to another file used only for production.

Thank you so much again

@dp1140a
Copy link
Author

dp1140a commented Jun 23, 2016

I think I found a fix to this issue. Instead of removing my server start code to another file I wrapped it it with:

if(server.info.started == 0){
    server.start(function (err) {
        if (err) {
            log.error(err);
        }
        log.info('Server running at ' + server.info.uri);
    });
}

I also did the same in the lib/forkme.js file:

if (http.info.created == 0) {
        http.start(function(err) {
          if (err) {
            process.send('Hapi server failed to start with error: ' + err);
          }

          if (!options.noasync) {
            process.send(null);
          }
        });
      }

That seems to have done the trick.

@athieriot
Copy link
Owner

It might not be a bad idea for me to add it to the forkme.js, I'll do some tests and keep you updated.

Thanks for the follow up !

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