CartoDB / CartoDB-SQL-API

CartoDB SQL API
BSD 3-Clause "New" or "Revised" License
63 stars 65 forks source link

Remove old abort checker #600

Closed dgaubert closed 5 years ago

dgaubert commented 5 years ago

Regarding this piece of code:

// client-abort.js
'use strict';

const http = require('http')
const port = 5555

const server = http.createServer((req, res) => {
  req.on('aborted', () => console.log(`Server: req aborted by client (req.aborted: ${req.aborted})`))
  setTimeout(() => res.end('chunk of data'), 100)
});

server.listen(port);

server.on('listening', () => {
  const req = http.get({ port });

  req.on('error', err => {
    console.error('Client: error', err)
    server.close()
  })

  setTimeout(() => req.abort(), 50)
})
$ node client-abort.js
Server: req aborted by client (req.aborted: true)
Client: error { Error: socket hang up
    at createHangUpError (_http_client.js:323:15)
    at Socket.socketCloseListener (_http_client.js:364:25)
    at Socket.emit (events.js:203:15)
    at TCP._handle.close (net.js:606:12) code: 'ECONNRESET' }

Instead, use a Node-ish mechanism to achieve the same goal

dgaubert commented 5 years ago

I understand your point, it makes sense for people that are not familiar with express/node ecosystem. I tend to define meaningful names to avoid large explanations in comments. I'm not a big fan of comments but for documentation. I'd like to use JSDoc or similar, however I didn't find the time to give it a try.