fix: clean up sockets on EADDRINUSE server close

__tests__/integration.js

@@ -12,7 +12,7 @@ function clone (json) {
   return JSON.parse(JSON.stringify(json))
 }
 
-function makeEvent (eventOverrides) {
+function makeEvent (eventOverrides = {}) {
   const baseEvent = clone(apiGatewayEvent)
   const multiValueHeaders = {
     ...baseEvent.multiValueHeaders,
@@ -403,26 +403,14 @@ describe('integration tests', () => {
 
     const response = await serverlessExpress.proxy({
       server: serverWithListenCallback,
-      event: makeEvent({})
+      event: makeEvent()
     })
 
     expect(response.statusCode).toBe(200)
     expect(serverListenCallback).toHaveBeenCalled()
     serverWithListenCallback.close()
   })
 
-  test('server.onError EADDRINUSE', async () => {
-    const serverWithSameSocketPath = serverlessExpress.createServer({ app: mockApp })
-    serverWithSameSocketPath._socketPathSuffix = server._socketPathSuffix
-
-    const response = await serverlessExpress.proxy({
-      server: serverWithSameSocketPath,
-      event: makeEvent({})
-    })
-    expect(response.statusCode).toBe(200)
-    serverWithSameSocketPath.close()
-  })
-
   test('Multiple headers of the same name (set-cookie)', async () => {
     const response = await serverlessExpress.handler(makeEvent({
       path: '/cookie',
@@ -445,9 +433,23 @@ describe('integration tests', () => {
     }))
   })
 
+  // NOTE: These must remain as the final tests as the EADDRINUSE test breaks
+  // the main `server` used by tests since the socket is deleted
+  // and the server.onClose also closes `server`
+  test('server.onError EADDRINUSE', async () => {
+    const serverWithSameSocketPath = serverlessExpress.createServer({ app: mockApp })
+    serverWithSameSocketPath._awsServerlessExpress.socketPath = server._awsServerlessExpress.socketPath
+
+    const response = await serverlessExpress.proxy({
+      server: serverWithSameSocketPath,
+      event: makeEvent()
+    })
+    expect(response.statusCode).toBe(200)
+    expect(serverWithSameSocketPath._awsServerlessExpress.socketPath).not.toBe(server._awsServerlessExpress.socketPath)
+    serverWithSameSocketPath.close()
+  })
+
   test('server.onClose', async () => {
-    // NOTE: this must remain as the final test as it closes `server`
-    await serverlessExpress.handler(makeEvent({}))
     server.on('close', () => {
       expect(server.listening).toBe(false)
     })

__tests__/unit.js

@@ -132,7 +132,9 @@ class MockResponse extends PassThrough {
 
 class MockServer {
   constructor (binaryMimeTypes = []) {
-    this._binaryMimeTypes = binaryMimeTypes
+    this._awsServerlessExpress = {
+      binaryMimeTypes
+    }
   }
 }
 

src/index.js

@@ -12,6 +12,7 @@
  * express or implied. See the License for the specific language governing
  * permissions and limitations under the License.
  */
+const fs = require('fs')
 const http = require('http')
 const { createLogger, format, transports } = require('winston')
 const {
@@ -20,15 +21,9 @@ const {
   makeResolver,
   getSocketPath
 } = require('./transport')
-const {
-  getRandomString
-} = require('./utils')
-const {
-  getEventFnsBasedOnEventSource
-} = require('./event-mappings')
-const {
-  getEventSourceBasedOnEvent
-} = require('./event-mappings/utils')
+const { getRandomString } = require('./utils')
+const { getEventFnsBasedOnEventSource } = require('./event-mappings')
+const { getEventSourceBasedOnEvent } = require('./event-mappings/utils')
 
 const currentLambdaInvoke = {}
 
@@ -48,16 +43,24 @@ function createServer ({
 }) {
   logger.debug('Creating HTTP server based on app...', { app, binaryMimeTypes })
   const server = http.createServer(app)
+  const socketPathSuffix = getRandomString()
+  const socketPath = getSocketPath({ socketPathSuffix })
+  server._awsServerlessExpress = {
+    socketPath,
+    binaryMimeTypes: binaryMimeTypes ? [...binaryMimeTypes] : []
+  }
 
-  server._socketPathSuffix = getRandomString()
-  server._binaryMimeTypes = binaryMimeTypes ? [...binaryMimeTypes] : []
   logger.debug('Created HTTP server', { server })
   server.on('error', (error) => {
     /* istanbul ignore else */
     if (error.code === 'EADDRINUSE') {
-      logger.warn(`Attempting to listen on socket ${getSocketPath({ socketPathSuffix: server._socketPathSuffix })}, but it's already in use. This is likely as a result of a previous invocation error or timeout. Check the logs for the invocation(s) immediately prior to this for root cause. If this is purely as a result of a timeout, consider increasing the function timeout and/or cpu/memory allocation. aws-serverless-express will restart the Node.js server listening on a new port and continue with this request.`)
-      server._socketPathSuffix = getRandomString()
-      return server.close(() => startServer({ server }))
+      logger.warn(`Attempting to listen on socket ${socketPath}, but it's already in use. This is likely as a result of a previous invocation error or timeout. Check the logs for the invocation(s) immediately prior to this for root cause. If this is purely as a result of a timeout, consider increasing the function timeout and/or cpu/memory allocation. aws-serverless-express will restart the Node.js server listening on a new port and continue with this request.`)
+      server._awsServerlessExpress.socketPath = getSocketPath({ socketPathSuffix: getRandomString() })
+
+      return server.close(() => {
+        fs.unlinkSync(error.address)
+        startServer({ server })
+      })
     } else {
       logger.error('aws-serverless-express server error: ', error)
     }

src/transport.js

@@ -1,11 +1,6 @@
 const http = require('http')
-const {
-  getEventFnsBasedOnEventSource
-} = require('./event-mappings')
-const {
-  getContentType,
-  isContentTypeBinaryMimeType
-} = require('./utils')
+const { getEventFnsBasedOnEventSource } = require('./event-mappings')
+const { getContentType, isContentTypeBinaryMimeType } = require('./utils')
 
 function forwardResponse ({
   server,
@@ -29,7 +24,7 @@ function forwardResponse ({
       logger.debug('contentType', { contentType })
       const isBase64Encoded = isContentTypeBinaryMimeType({
         contentType,
-        binaryMimeTypes: server._binaryMimeTypes
+        binaryMimeTypes: server._awsServerlessExpress.binaryMimeTypes
       })
       const body = bodyBuffer.toString(isBase64Encoded ? 'base64' : 'utf8')
       const successResponse = eventResponseMapperFn({
@@ -97,10 +92,9 @@ function forwardRequestToNodeServer ({
   logger.debug('Forwarding request to application...')
   const eventResponseMapperFn = eventFns.response
   try {
-    const socketPath = getSocketPath({ socketPathSuffix: server._socketPathSuffix })
     const { body, ...requestOptions } = eventFns.request({ event })
     logger.debug('requestOptions', requestOptions)
-    const req = http.request({ socketPath, ...requestOptions }, (response) => forwardResponse({
+    const req = http.request({ socketPath: server._awsServerlessExpress.socketPath, ...requestOptions }, (response) => forwardResponse({
       server,
       response,
       resolver,
@@ -134,7 +128,7 @@ function forwardRequestToNodeServer ({
 }
 
 function startServer ({ server }) {
-  return server.listen(getSocketPath({ socketPathSuffix: server._socketPathSuffix }))
+  return server.listen(server._awsServerlessExpress.socketPath)
 }
 
 function getSocketPath ({ socketPathSuffix }) {