fix(dev): close the listener when setup fails after binding

Author: danielroe

packages/nuxt-cli/src/dev/listen.ts

@@ -178,112 +178,128 @@ export async function listen(handler: RequestListener, options: ListenOptions =
 // takes the whole dev process down.
 server.on('error',error=>logger.error(`Dev server error: ${error.message}`))
 
-constaddress=server.address()asAddressInfo
-constprotocol=certificate ? 'https' : 'http'
-constbaseURL=options.baseURL||'/'
-constformatURL=(host: string)=>formatDisplayURL(protocol,host,address.port,baseURL)
-
-constanyHost=ANY_HOSTS.has(hostname)
-consturl=formatURL(anyHost ? 'localhost' : hostname)
-
+// Set inside `createListener`, so a failure after the tunnel is up can still
+// tear down the cloudflared process rather than leaking it until exit.
 lettunnel: Tunnel|undefined
-if(options.tunnel){
-const{ startTunnel }=awaitimport('./tunnel')
-tunnel=awaitstartTunnel(`${protocol}://localhost:${address.port}`,!!certificate)
+
+try{
+returnawaitcreateListener()
+}
+catch(error){
+awaitPromise.all([
+tunnel?.close().catch(()=>{}),
+closeServer(server).catch(()=>{}),
+])
+throwerror
 }
 
-consttunnelURL=tunnel?.url&&tunnel.url+baseURL
-constportless=resolvePortlessURLs()
-constportlessURL=portless.url&&portless.url+baseURL
-constportlessShareURL=portless.shareURL&&portless.shareURL+baseURL
-conststackblitzURL=resolveStackblitzURL()
+asyncfunctioncreateListener(): Promise<Listener>{
+constaddress=server.address()asAddressInfo
+constprotocol=certificate ? 'https' : 'http'
+constbaseURL=options.baseURL||'/'
+constformatURL=(host: string)=>formatDisplayURL(protocol,host,address.port,baseURL)
 
-functiongetURLs(): ListenURL[]{
-consturls: ListenURL[]=[]
-if(tunnelURL){
-urls.push({url: tunnelURL,type: 'tunnel'})
-}
-for(constportlessURLofportless.all){
-urls.push({url: portlessURL+baseURL,type: 'public'})
-}
-if(stackblitzURL){
-urls.push({url: stackblitzURL,type: 'public'})
+constanyHost=ANY_HOSTS.has(hostname)
+consturl=formatURL(anyHost ? 'localhost' : hostname)
+
+if(options.tunnel){
+const{ startTunnel }=awaitimport('./tunnel')
+tunnel=awaitstartTunnel(`${protocol}://localhost:${address.port}`,!!certificate)
 }
-if(anyHost){
-urls.push({url: formatURL('localhost'),type: 'local'})
-for(constaddressofgetNetworkAddresses()){
-urls.push({url: formatURL(address),type: 'network'})
+
+consttunnelURL=tunnel?.url&&tunnel.url+baseURL
+constportless=resolvePortlessURLs()
+constportlessURL=portless.url&&portless.url+baseURL
+constportlessShareURL=portless.shareURL&&portless.shareURL+baseURL
+conststackblitzURL=resolveStackblitzURL()
+
+functiongetURLs(): ListenURL[]{
+consturls: ListenURL[]=[]
+if(tunnelURL){
+urls.push({url: tunnelURL,type: 'tunnel'})
 }
+for(constportlessURLofportless.all){
+urls.push({url: portlessURL+baseURL,type: 'public'})
+}
+if(stackblitzURL){
+urls.push({url: stackblitzURL,type: 'public'})
+}
+if(anyHost){
+urls.push({url: formatURL('localhost'),type: 'local'})
+for(constaddressofgetNetworkAddresses()){
+urls.push({url: formatURL(address),type: 'network'})
+}
+}
+else{
+urls.push({ url,type: 'local'})
+}
+returnurls
 }
-else{
-urls.push({ url,type: 'local'})
-}
-returnurls
-}
 
-// The StackBlitz URL points at the editor rather than at a host another
-// device can open, so it is not a QR code candidate.
-constshareableURL=options.publicURL||tunnelURL||portlessShareURL||portlessURL
-constpublicURL=shareableURL||stackblitzURL
-
-constqrURL=options.qr===false
- ? undefined
- : shareableURL
-||getURLs().find(({ type })=>type==='network')?.url
-||(options.qr ? url : undefined)
-
-functionshowURLs({ qr =false}: {qr?: boolean}={}): void{
-consturls=getURLs()
-constlabels={local: 'Local:',network: 'Network:',tunnel: 'Tunnel:',public: 'Public:'}asconst
-constlabelColors={local: 'green',network: 'magenta',tunnel: 'cyan',public: 'magenta'}asconst
-constlines: string[]=[]
-constline=(color: (text: string)=>string,label: string,value: string,isQR: boolean)=>
-` ${color('➜')}${styleText('bold',color(label.padEnd(10)))}${value}${isQR ? styleText('gray',' [QR code]') : ''}`
-for(const{url: displayURL, type }ofurls){
-lines.push(line(text=>styleText(labelColors[type],text),labels[type],styleText('cyan',displayURL),qr&&displayURL===qrURL))
-}
-if(!anyHost&&!tunnel&&!portless.url&&!stackblitzURL){
-constisolated=LOOPBACK_HOSTS.has(hostname) ? detectIsolatedEnvironment() : undefined
-consthint=isolated
- ? `use ${styleText('white','--host')} to reach this server from outside ${isolated}`
- : `use ${styleText('white','--host')} to expose`
-lines.push(line(text=>styleText('magenta',text),'Network:',styleText('gray',hint),false))
-}
-if(publicURL&&publicURL!==url&&!urls.some(entry=>entry.url===publicURL)){
-lines.push(line(text=>styleText('magenta',text),'Public:',styleText('cyan',publicURL),qr&&publicURL===qrURL))
+// The StackBlitz URL points at the editor rather than at a host another
+// device can open, so it is not a QR code candidate.
+constshareableURL=options.publicURL||tunnelURL||portlessShareURL||portlessURL
+constpublicURL=shareableURL||stackblitzURL
+
+constqrURL=options.qr===false
+ ? undefined
+ : shareableURL
+||getURLs().find(({ type })=>type==='network')?.url
+||(options.qr ? url : undefined)
+
+functionshowURLs({ qr =false}: {qr?: boolean}={}): void{
+consturls=getURLs()
+constlabels={local: 'Local:',network: 'Network:',tunnel: 'Tunnel:',public: 'Public:'}asconst
+constlabelColors={local: 'green',network: 'magenta',tunnel: 'cyan',public: 'magenta'}asconst
+constlines: string[]=[]
+constline=(color: (text: string)=>string,label: string,value: string,isQR: boolean)=>
+` ${color('➜')}${styleText('bold',color(label.padEnd(10)))}${value}${isQR ? styleText('gray',' [QR code]') : ''}`
+for(const{url: displayURL, type }ofurls){
+lines.push(line(text=>styleText(labelColors[type],text),labels[type],styleText('cyan',displayURL),qr&&displayURL===qrURL))
+}
+if(!anyHost&&!tunnel&&!portless.url&&!stackblitzURL){
+constisolated=LOOPBACK_HOSTS.has(hostname) ? detectIsolatedEnvironment() : undefined
+consthint=isolated
+ ? `use ${styleText('white','--host')} to reach this server from outside ${isolated}`
+ : `use ${styleText('white','--host')} to expose`
+lines.push(line(text=>styleText('magenta',text),'Network:',styleText('gray',hint),false))
+}
+if(publicURL&&publicURL!==url&&!urls.some(entry=>entry.url===publicURL)){
+lines.push(line(text=>styleText('magenta',text),'Public:',styleText('cyan',publicURL),qr&&publicURL===qrURL))
+}
+// eslint-disable-next-line no-console
+console.log(`${qr ? '' : '\n'}${lines.join('\n')}\n`)
 }
-// eslint-disable-next-line no-console
-console.log(`${qr ? '' : '\n'}${lines.join('\n')}\n`)
-}
 
-if(options.showURL!==false){
-if(qrURL){
-awaitprintQRCode(qrURL)
+if(options.showURL!==false){
+if(qrURL){
+awaitprintQRCode(qrURL)
+}
+showURLs({qr: !!qrURL})
 }
-showURLs({qr: !!qrURL})
-}
 
-if(options.clipboard){
-awaitcopyURL(publicURL||url)
-}
+if(options.clipboard){
+awaitcopyURL(publicURL||url)
+}
 
-if(options.open){
-openBrowser(options.openURL ? resolveOpenURL(options.openURL,url) : url)
-}
+if(options.open){
+openBrowser(options.openURL ? resolveOpenURL(options.openURL,url) : url)
+}
 
-return{
- url,
- publicURL,
- qrURL,
- address,
- server,
-https: certificate,
- getURLs,
- showURLs,
-close: ()=>Promise.all([
-tunnel?.close(),
-closeServer(server),
-]).then(()=>{}),
+return{
+ url,
+ publicURL,
+ qrURL,
+ address,
+ server,
+https: certificate,
+ getURLs,
+ showURLs,
+close: ()=>Promise.all([
+tunnel?.close(),
+closeServer(server),
+]).then(()=>{}),
+}
 }
 }
 

packages/nuxt-cli/test/unit/listen.spec.ts

@@ -290,6 +290,43 @@ describe('listener.close', () => {
 vi.doUnmock('../../src/dev/tunnel')
 })
 
+it('should close the tunnel when setup fails after it started',async()=>{
+constcloseTunnel=vi.fn(()=>Promise.resolve())
+vi.doMock('../../src/dev/tunnel',()=>({
+startTunnel: async()=>({url: 'https://example.test',close: closeTunnel}),
+}))
+vi.doMock('uqr',()=>({
+renderUnicodeCompact: ()=>{
+thrownewError('qr unavailable')
+},
+}))
+
+awaitexpect(listen((_req,res)=>res.end('ok'),{port: 0,hostname: '127.0.0.1',tunnel: true,qr: true})).rejects.toThrow('qr unavailable')
+
+expect(closeTunnel).toHaveBeenCalledTimes(1)
+vi.doUnmock('../../src/dev/tunnel')
+vi.doUnmock('uqr')
+})
+
+it('should release the port when setup fails after binding',async()=>{
+vi.doMock('../../src/dev/tunnel',()=>({
+startTunnel: async()=>{
+thrownewError('tunnel unavailable')
+},
+}))
+
+constprobe=awaitlisten((_req,res)=>res.end('ok'),{port: 0,hostname: '127.0.0.1',showURL: false})
+constport=probe.address.port
+awaitprobe.close()
+
+awaitexpect(listen((_req,res)=>res.end('ok'),{ port,hostname: '127.0.0.1',showURL: false,strictPort: true,tunnel: true})).rejects.toThrow('tunnel unavailable')
+vi.doUnmock('../../src/dev/tunnel')
+
+constlistener=awaitlisten((_req,res)=>res.end('ok'),{ port,hostname: '127.0.0.1',showURL: false,strictPort: true})
+expect(listener.address.port).toBe(port)
+awaitlistener.close()
+})
+
 it('should let an in-flight request finish',async()=>{
 letrespond: (()=>void)|undefined
 constlistener=awaitlisten((_req,res)=>{