prolic / HumusAmqp

PHP 7.4 AMQP library
https://humusamqp.readthedocs.io/
MIT License
76 stars 17 forks source link

json rpc server returns trace if enabled #60

Closed prolic closed 6 years ago

prolic commented 6 years ago

see also: https://github.com/prolic/HumusAmqp/pull/56

This is a small BC as the Error Interface now has an additional method. Usually it would not be expected to have an implementation of this Error Interface in userland code, so maybe we can introduce it without new major version.

Thoughts? @oqq @thomasvargiu @bweston92

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.1%) to 92.801% when pulling beb8574164e59e3e6c659bc944398c0784f07941 on json_rpc_trace into 73a777bed565a072f1653fd50a1680fb4bb48080 on master.

oqq commented 6 years ago

This change does not match the JSON-RPC 2.0 Spec for the error object. How I read the rfc, this response should only contain code, message and data.

Maybe the trace should be in data.

prolic commented 6 years ago

The trace is only meant for debugging in development mode, and should not be turned on in production, that's why I think it's okay to add it as additional field to the error object. The purpose of data is to show example data, how a good request looks like, also changing the data field would lead to BC break on consumer side, that's why we should not add it to data.

oqq commented 6 years ago

http://www.jsonrpc.org/specification#error_object

data A Primitive or Structured value that contains additional information about the error. This may be omitted. The value of this member is defined by the Server (e.g. detailed error information, nested errors etc.).

The data field is defined exactly for this behaivor.. adding some more info about the error.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.4%) to 91.333% when pulling 78c42ea7255a0675e26ac180ca5b2e66972f148e on json_rpc_trace into 73a777bed565a072f1653fd50a1680fb4bb48080 on master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.3%) to 91.365% when pulling 909eb0e3e2591c9e443668cc9305b6a4dddc21cf on json_rpc_trace into 73a777bed565a072f1653fd50a1680fb4bb48080 on master.