cargomedia / cm-janus

UNMAINTAINED. cm/janus bridge
MIT License
2 stars 6 forks source link

Improve http server requests' logging #262

Closed tomaszdurka closed 8 years ago

tomaszdurka commented 8 years ago
vogdb commented 8 years ago

@tomaszdurka please review

vogdb commented 8 years ago
  • intercept context-data sent from CM application (similar to websocket)

@tomaszdurka please tell the format of this context-data. When we receive a response from CM we take its 'success' field as the result.

return response['success']['result'];

Under what field name we should expect CM context-data?

tomaszdurka commented 8 years ago

Yes, we need issue for CM to send this data. Just created it https://github.com/cargomedia/cm/issues/2154.

Let's expect this data to come in context get parameter. Exactly the same as in websocket server https://github.com/cargomedia/cm-janus/blob/master/lib/janus/connection.js#L166 This way we may be consistent for all types of requests (GET/POST, etc).

vogdb commented 8 years ago

@tomaszdurka please look. Not sure about this delete response['context']; but otherwise we log it twice.

codecov-io commented 8 years ago

Current coverage is 90.36%

Merging #262 into 0.8.5 will decrease coverage by <.01%

@@              0.8.5       #262   diff @@
==========================================
  Files            34         34          
  Lines          1316       1318     +2   
  Methods         324        324          
  Messages          0          0          
  Branches        142        143     +1   
==========================================
+ Hits           1190       1191     +1   
- Misses          126        127     +1   
  Partials          0          0          

Powered by Codecov. Last updated by 8e5228c...2e9f9c7

vogdb commented 8 years ago

@tomaszdurka please review

tomaszdurka commented 8 years ago

Looks good ;)

vogdb commented 8 years ago

Will merge and release tomorrow