getappmap / appmap-ruby

AppMap client agent for Ruby
https://appland.org
Other
100 stars 13 forks source link

fix: Don't emit invalid HTTP status #347

Closed kgilpin closed 11 months ago

kgilpin commented 11 months ago

During websocket upgrade, the Rack middleware is emitting status -1:

https://github.com/rails/rails/blob/febd21da3438d190f0c6ce296a114baee983d1e1/actioncable/lib/action_cable/connection/client_socket.rb#L71C23-L71C23

This is updated elsewhere:

https://github.com/faye/websocket-driver-ruby/blob/58d4ed742749dfb3b4f568df36a5365088361220/lib/websocket/driver/draft76.rb#L61

It's better not to emit a status code at all than to emit an invalid one. Downstream dependencies of AppMap will need to anticipate that the HTTP server response data may not be complete, and either auto-complete the data or discard it.

A more complete solution would be to hook the Websocket driver and update the status code (should be 101). But given that's going directly to the wire, doesn't seem realistic for now.

OpenAPI generator will need to discard HTTP server response examples with missing or invalid status codes.

Fixes https://github.com/getappmap/appmap-ruby/issues/346

Related to https://github.com/getappmap/appmap-js/pull/1459

kgilpin commented 11 months ago

:tada: This PR is included in version 0.102.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: