Decoding issue for 40010 Error (Invalid Channel Name)

[umair-ably]

Publishing a message on an invalid channel causes the SDK to fail to decode the error response appropriately. Example error log:

####### TEXT ��error��message�resource: invalid name ":"�href� https://help.ably.io/error/40010�code��J�statusCode��
Traceback (most recent call last):
  File "/home/void/.pyenv/versions/klash/lib/python3.11/site-packages/ably/sync/util/exceptions.py", line 38, in raise_for_response
    json_response = response.json()
                    ^^^^^^^^^^^^^^^
  File "/home/void/.pyenv/versions/klash/lib/python3.11/site-packages/httpx/_models.py", line 766, in json
    return jsonlib.loads(self.content, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/void/.pyenv/versions/3.11.4/lib/python3.11/json/__init__.py", line 341, in loads
    s = s.decode(detect_encoding(s), 'surrogatepass')
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x81 in position 0: invalid start byte

Reproduction Steps
Publish a message to the ":" channel (or any other invalid channel)

Expected Behaviour
The SDK should correctly decode and return the 40010 error.

Additional Notes:
It's worth triggering a couple of other error codes within the SDK to see if they are decoded appropriately, just to ensure this decoding issue is not a widespread problem.

┆Issue is synchronized with this Jira Task by Unito

[lmars]

@umair-ably I assume that error parsing code should be using the same logic as when decoding successful responses:

ably-python/ably/http/http.py

Lines 100 to 105 in dff73cd

	content_type = self.__response.headers.get('content-type')
	if isinstance(content_type, str):
	if content_type.startswith('application/x-msgpack'):
	return msgpack.unpackb(content)
	elif content_type.startswith('application/json'):
	return self.__response.json()

[umair-ably]

@lmars I'd assume so too with it failing to decode to json. This is the raw text printed out before it attempts to decode it:

####### TEXT ��error��message�resource: invalid name ":"�href� https://help.ably.io/error/40010�code��J�statusCode��

[lmars]

@umair-ably yes, so that is msgpack, evident from the error "can't decode byte 0x81 in position 0", which from my many hours of inspecting raw msgpack is the start of a msgpack encoded map containing one key-value pair.

[umair-ably]

@lmars I'm not sure I follow, are you implying the user is incorrectly assuming they will get a json response when instead it's actually a msgpack response?

[AlexanderArvidsson]

Hi, original reporter of the issue here:

We're using msgpack in frontend via the useBinaryProtocol, but our backend Python SDK isn't configured to use msgpack. I'm assuming that our Ably app in the Ably cloud is configured to output everything to msgpack, and the python SDK isn't handling that on errors.

I think @lmars is on the right track with the content type switch, since the error response handler just assumes Json at all times, as opposed to conditionally using msgpack or json.

[lmars]

@umair-ably apologies, let me explain.

The realtime system supports returning responses encoded as either JSON or msgpack, and the SDK indicates which encoding it wants by setting the HTTP Accept header or the format query param, which in turn is controlled by the useBinaryProtocol client option, true for msgpack, false for JSON.

It seems to me that within the SDK, it calls AblyException.raise_for_response in various places, for example here, but that isn't taking into account what encoding the SDK is expecting and always assuming JSON, which is incorrect.

I think this is therefore a bug in the SDK, specifically that raise_for_response should be checking whether the response is msgpack or JSON and decoding accordingly, rather than assuming JSON.

[umair-ably]

That makes perfect sense - thanks @lmars! I'll get this prioritised in our next sprint planning

[lmars]

I've proposed a fix for this in #571, since this came up as a problem in some internal testing too.

[AlexanderArvidsson]

Hi @lmars, can we get this fix released to Pip?

[lmars]

Hey @AlexanderArvidsson, I've raised this internally and we'll get it released asap.