jishi / node-sonos-http-api

An HTTP API bridge for Sonos easing automation. Hostable on any node.js capable device, like a raspberry pi or similar.
http://jishi.github.io/node-sonos-http-api/
MIT License
1.85k stars 460 forks source link

"say" crashing node-sonos-http-api - Can't set headers after they are sent #163

Closed 2MuchTech closed 8 years ago

2MuchTech commented 8 years ago

It's been a few weeks since I did anything with the "say" command, and it used to work (although not 100% reliably, as there are known timing issues that I know you're working on with your rewritten base code). Anyway, today "say" is crashing node-sonos. I think the only major thing that's changed in my node-sonos since I last successfully used the "say" command is that I now have basic authentication enabled in node-sonos (because I'm using Amazon Echo to control node-sonos). Everything else except "say" seems to be working fine.

Anyway, here's a dump I get from Webstorm when I try to use the "say" command. It DOES play the message, but then node-sonos crashes. Any ideas?

Using cached tts message file: 4cf30e145455e1bd47a5d840c192e5dcc6d52b4f-en-au.mp3
info: applying preset players=[roomName=Office, volume=40], state=playing, playMode=NORMAL, uri=http://192.168.1.150:5005/tts/4cf30e145455e1bd47a5d840c192e5dcc6d52b4f-en-au.mp3
debug: ungrouping Office to prepare for grouping
error: error occured on soap request read ECONNRESET
error:  code=ECONNRESET, errno=ECONNRESET, syscall=read
error:  code=ECONNRESET, errno=ECONNRESET, syscall=read
error: subscribe failed uuid:RINCON_000E5812918201400_sub0000005482 /MediaRenderer/AVTransport/Event 412
error: subscribe failed uuid:RINCON_000E5812918201400_sub0000005483 /MediaRenderer/RenderingControl/Event 412
STOPPED http://192.168.1.150:5005/tts/4cf30e145455e1bd47a5d840c192e5dcc6d52b4f-en-au.mp3
error: subscribe failed uuid:RINCON_000E5812918201400_sub0000005484 /MediaRenderer/GroupRenderingControl/Event 412
error: subscribe failed uuid:RINCON_000E5812918201400_sub0000005485 /MediaServer/ContentDirectory/Event 412
_http_outgoing.js:335
    throw new Error('Can\'t set headers after they are sent.');
    ^

Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:335:11)
    at /Users/xxxx/WebstormProjects/node-sonos-http-api/server.js:54:15
    at Server.finish (/Users/xxxx/WebstormProjects/node-sonos-http-api/node_modules/node-static/lib/node-static.js:128:21)
    at Server.serve.finish (/Users/xxxx/WebstormProjects/node-sonos-http-api/node_modules/node-static/lib/node-static.js:170:14)
    at /Users/xxxx/WebstormProjects/node-sonos-http-api/node_modules/node-static/lib/node-static.js:341:13
    at streamFile (/Users/xxxx/WebstormProjects/node-sonos-http-api/node_modules/node-static/lib/node-static.js:387:13)
    at ReadStream.<anonymous> (/Users/xxxx/WebstormProjects/node-sonos-http-api/node_modules/node-static/lib/node-static.js:380:17)
    at emitNone (events.js:67:13)
    at ReadStream.emit (events.js:166:7)
    at fs.js:1776:14
    at FSReqWrap.oncomplete (fs.js:82:15)

Process finished with exit code 1
2MuchTech commented 8 years ago

I've done a little debugging, but I still don't know how to fix the problem. The issue only occurs when basic auth is enabled. For some reason the requestHandler function in server.js is getting called three (3) times for each "say" command invoked by a browser. The first 2 timescredentials gets assigned correctly, but on the third call credentials is left undefined after the call to auth(req). The subsequent res.setHeader function then generates the error message I noted in my original post and the program aborts.

I have noticed that on the third call to the requestHandler function, req.headers contains an entry connection = "close", so I'm going to try to modify the code to look for this and return from the function. Since I don't know what the heck is supposed to happen on this third call, I'm guessing my "solution" isn't going to be correct even if it sidesteps the crash bug. I'd appreciate some suggestions for the proper way to handle this. Thanks!

2MuchTech commented 8 years ago

Upon further review, it appears the variable err is undefined in the third call to the requestHandler function, so I ended up just moving some of your existing code around and it seems to work now. Is this a legitimate fix? Here's the way I have the code now:

      // If error, route it.
      if (!err) {
        return;
      }

      if (settings.auth) {
        var credentials = auth(req);

        if (!credentials || credentials.name !== settings.auth.username || credentials.pass !== settings.auth.password) {
          res.statusCode = 401;
          res.setHeader('WWW-Authenticate', 'Basic realm="Access Denied"');
          res.end('Access denied');
          return;
        }
      }

      if (req.method === 'GET') {
        api.requestHandler(req, res);
      }
jishi commented 8 years ago

Sorry about the late answer. This was a good catch, what I believe happens is that the request from Sonos for fetching the TTS file, crashes the api. Your fix resolves it, but it also exposes any static file (within the static folder), including tts files etc without authentication. This might not be desirable, but is probably the best case for now.

Sonos on the other hand might handle authenticated requests if they would be part of the url, I haven't tested it.

jishi commented 8 years ago

Fixed in b588a84476f9f59a82422bedf091828b93d5f08e