-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
add pretty message if port already in use(#927) #932
Changes from 8 commits
0efd6f9
30e77d1
d675deb
cb9b3b3
9f6ccce
f55b901
8de2b51
9404483
0278e8b
6d7945c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
import 'source-map-support/register' | ||
import { resolve, join } from 'path' | ||
import parseArgs from 'minimist' | ||
import { existsSync } from 'fs' | ||
import { existsSync, readFileSync } from 'fs' | ||
import Server from '../server' | ||
import { printAndExit } from '../lib/utils' | ||
|
||
|
@@ -61,6 +61,16 @@ srv.start(argv.port) | |
} | ||
}) | ||
.catch((err) => { | ||
console.error(err) | ||
process.exit(1) | ||
if (err.code === 'EADDRINUSE') { | ||
let errorMessage = `Port ${argv.port} is already in use.` | ||
try { | ||
const appPackage = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use pkg-up to find the package.json. Dir could be a directory inside root. In that case, there will be no |
||
const nextTuple = Object.entries(appPackage.scripts).find(i => i[1] === 'next') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happen when we don't find a one. Then do nothing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also it's possible for users to give some additional stuff to the next command like: |
||
errorMessage += `\nUse \`npm run ${nextTuple[0]} -- -p <some other port>\`.` | ||
} catch (e) { } | ||
console.error(errorMessage) | ||
} else { | ||
console.error(err) | ||
} | ||
process.nextTick(() => process.exit(1)) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,10 +119,9 @@ export default class Server { | |
await this.prepare() | ||
this.http = http.createServer(this.getRequestHandler()) | ||
await new Promise((resolve, reject) => { | ||
this.http.listen(port, (err) => { | ||
if (err) return reject(err) | ||
resolve() | ||
}) | ||
this.http.on('error', reject) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why we need to do this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This code does not catch the error when next starting listen port which already in use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. |
||
this.http.on('listening', () => resolve()) | ||
this.http.listen(port) | ||
}) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need a try catch block here.