isServerRunning hides application errors

[ferares]

Hi,

So I was getting the following error: Server docroot returned 500-level response. Please check your configuration for possible errors. and, after trying some things out, I finally decided to comment out the reject on line 31 of index.js and replace it with a resolve call, after doing that I could see that the cause of the error was an undefined function, so it was an application problem not a server, or configuration one.

I believe the error message is misleading and that errors of this type should still resolve the promise instead of reject it. Maybe I'm missing the reason why you're checking for if (statusCodeType === 5) and calling reject instead of just resolving and letting the user see the error for themselves.

Please let me know your thoughts on this and thank you for your time and effort 🙌.

[sindresorhus]

It was added in sindresorhus/grunt-php@4dd0566. I think it's useful to catch errors early as not everyone is using this interactively. I do agree the error message should be made clearer. Maybe we could do a GET request on a 5xx response to get the actual error and show that.

[ferares]

Making a GET request seems like a good idea, I've done that with the following code but it seems kinda messy, maybe there's a cleaner way to do it (?)

	// Check when the server is ready. Tried doing it by listening
	// to the child process `data` event, but it's not triggered...
	try {
		await isServerRunning(options.hostname, options.port, pathname);
	} catch (error) {
		console.error(error)
		if (error.cause) new Error(error) // Request error
		// 500 status error
		subprocess.stderr.on('data', (data) => process.stdout.write(data));
		await (new Promise((resolve, reject) => {
			http.request({
				method: 'GET',
				hostname: options.hostname,
				port: options.port,
				path: pathname
			}, response => resolve).end();
		}));
		new Error(error)
	}

Note that I've added a "cause" to the error being thrown when the request fails before getting a response, that's to differentiate it from the error thrown by a 500 status code:

reject(new Error(`Could not start the PHP server: ${error.message}`, { cause: error }))

If you think it's ok I can send a PR with this solution