Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: posting multipart-formdata and integer #920

Closed
pylover opened this issue Jun 10, 2016 · 22 comments · Fixed by #1580
Closed

client: posting multipart-formdata and integer #920

pylover opened this issue Jun 10, 2016 · 22 comments · Fixed by #1580
Assignees
Labels

Comments

@pylover
Copy link

pylover commented Jun 10, 2016

the multipart-formdata is not working with integer values, i spend many times to find-out what is the problem:

Actual behaviour

Traceback (most recent call last):
  File "/home/vahid/workspace/aiohttp/aiohttp/client_reqrep.py", line 390, in write_bytes
    result = stream.send(value)
  File "/home/vahid/workspace/aiohttp/aiohttp/helpers.py", line 188, in _gen_form_data
    yield from self._writer.serialize()
  File "/home/vahid/workspace/aiohttp/aiohttp/multipart.py", line 958, in serialize
    yield from part.serialize()
  File "/home/vahid/workspace/aiohttp/aiohttp/multipart.py", line 759, in serialize
    yield from self._maybe_encode_stream(self._serialize_obj())
  File "/home/vahid/workspace/aiohttp/aiohttp/multipart.py", line 772, in _serialize_obj
    return self._serialize_default(obj)
  File "/home/vahid/workspace/aiohttp/aiohttp/multipart.py", line 804, in _serialize_default
    raise TypeError('unknown body part type %r' % type(obj))
TypeError: unknown body part type <class 'int'>

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/opt/pycharm-2016.1.1/helpers/pydev/pydevd.py", line 1531, in <module>
    globals = debugger.run(setup['file'], None, None, is_module)
  File "/opt/pycharm-2016.1.1/helpers/pydev/pydevd.py", line 938, in run
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/opt/pycharm-2016.1.1/helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/home/vahid/workspace/hazelnut/practice/formdata.py", line 26, in <module>
    loop.run_until_complete(main())
  File "/usr/local/lib/python3.5/asyncio/base_events.py", line 337, in run_until_complete
    return future.result()
  File "/usr/local/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
  File "/usr/local/lib/python3.5/asyncio/tasks.py", line 241, in _step
    result = coro.throw(exc)
  File "/home/vahid/workspace/hazelnut/practice/formdata.py", line 21, in main
    await session.post(url, data=data)
  File "/home/vahid/workspace/aiohttp/aiohttp/client.py", line 537, in __await__
    resp = yield from self._coro
  File "/home/vahid/workspace/aiohttp/aiohttp/client.py", line 191, in _request
    yield from resp.start(conn, read_until_eof)
  File "/home/vahid/workspace/aiohttp/aiohttp/client_reqrep.py", line 619, in start
    message = yield from httpstream.read()
  File "/home/vahid/workspace/aiohttp/aiohttp/streams.py", line 592, in read
    result = yield from super().read()
  File "/home/vahid/workspace/aiohttp/aiohttp/streams.py", line 447, in read
    yield from self._waiter
  File "/usr/local/lib/python3.5/asyncio/futures.py", line 358, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/local/lib/python3.5/asyncio/tasks.py", line 290, in _wakeup
    future.result()
  File "/usr/local/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
aiohttp.errors.ClientRequestError: Can not write request body for http://example.org/

Steps to reproduce

import asyncio
import aiohttp
import io


sample_data = {
    'host': 'localhost',
    'label': 'DefaultNode',
    'quality': 100,
    'capacity': 100000,
}

async def main():
    url = 'http://example.org'
    data = aiohttp.FormData()
    data.add_field('file', io.StringIO('Sample text file'), content_type='text/plain')
    for k, v in sample_data.items():
        data.add_field(k, v)

    with aiohttp.ClientSession() as session:
        await session.post(url, data=data)


if __name__ == '__main__':
    loop = asyncio.get_event_loop()
    loop.run_until_complete(main())

My environment

os: Ubuntu 14.04
python 3.5.1
aiohttp: master: f180199

@asvetlov
Copy link
Member

See also #761

@kxepal we can add more obvious error into _serialize_default just for integer/float case.
Opinions?

@kxepal
Copy link
Member

kxepal commented Jun 11, 2016

@asvetlov
I think the error itself is pretty clear, but general issue is that serialization is too much lazy which makes hard to catch the moment when issue actually happens. For instance, if an exception was raised on add_field call it would be much easy to find out why and for what exactly field it happened.

Let me rethink for MultipartWriter works, may be we can make it more friendly.

@asvetlov
Copy link
Member

Exception in add_field work for me

@pylover
Copy link
Author

pylover commented Jun 13, 2016

Thanks a lot. but some frameworks trying to convert the given value to str

@kxepal
Copy link
Member

kxepal commented Jun 14, 2016

@pylover
It's tricky. If we do convert everything to string, we can produce unwanted results. Let's pick int. It looks pretty safe to do just str(intval), but how to be with bool type? They are int-like, but it depends on server in which way to handle them: True or true or 1 or something else?

With other types there is more or less same story. And if we add support of one Python base type, why not to add the rest? But how?

For now we expect value as str and only and that's gives pretty consistent behavior and expectations (except the moment when you get the error about bad value - that's really unfriendly).

@asvetlov
Copy link
Member

@kxepal do you have a time for refactoring or prefer to close the issue without a fix?

@kxepal
Copy link
Member

kxepal commented Jul 17, 2016

@asvetlov Yes, I do, will take care about this.

@kxepal kxepal self-assigned this Jul 17, 2016
@pylover
Copy link
Author

pylover commented Aug 4, 2016

Thanks a lot.

@fafhrd91
Copy link
Member

@kxepal any news?

@pylover
Copy link
Author

pylover commented Jan 26, 2017

@kxepal, Currently i completely agree with you, so i think the python's __repr__ is enought, This is what the other frameworks do.

@kxepal
Copy link
Member

kxepal commented Jan 26, 2017

@pylover You mean apply repr() as default serializer?

@pylover
Copy link
Author

pylover commented Jan 27, 2017

@kxepal , Yes, but applying an IOC mechanism to allow users to register their own preferred callable as serializer is usefull

@kxepal
Copy link
Member

kxepal commented Jan 27, 2017

@pylover
This worries me since you will notice the problem only after you start to send requests with something like 'ParallelCollectionRDD[0] at parallelize at PythonRDD.scala:423'. Lucky you if backend will not accept anything of such, but what if it will?

@pylover
Copy link
Author

pylover commented Jan 27, 2017

@kxepal , What about using __str__ ( str() )?

I think the str() function is better choice.

@kxepal
Copy link
Member

kxepal commented Jan 28, 2017

@pylover Yes, str() is a right choice, but it doesn't solves general issue I previously mentioned.

@pylover
Copy link
Author

pylover commented Jan 28, 2017

I don't get your meaning from previous comment: 'parallel......'. Why backend should accept such thing? And please mention a real world example to demonstrate the problem

@kxepal
Copy link
Member

kxepal commented Jan 28, 2017

@pylover
Actually, now there is no any realworld problem, because multipart will not implicitly serialize any values to string (: I'm trying say that such default serialization will cause the problems. One of them I demostrated: implicit serialization in the way you don't actually want.

@pylover
Copy link
Author

pylover commented Jan 28, 2017

@kxepal , So, raising ValueError for any non-string value?

@kxepal
Copy link
Member

kxepal commented Jan 28, 2017

@pylover We already have TypeError for that case - a bit more specific.

@pylover
Copy link
Author

pylover commented Jan 28, 2017

@kxepal , Yes, you are true. You can replace it to ValueError, to tell the user to serialize their data. Or wathever you want. At all, i prefer to stringify whatever received.

@pylover
Copy link
Author

pylover commented Feb 3, 2017

Thank's a lot

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants