Skip to content

Python2.7 - AssertionError: Status must be a string - #679

Closed
Geethree wants to merge 1 commit into
prometheus:masterfrom
Geethree:master
Closed

Geethree wants to merge 1 commit into
prometheus:masterfrom
Geethree:master

Conversation

@Geethree

@Geethree Geethree commented Jul 1, 2021

Copy link
Copy Markdown

TL;DR

As noted #662. When running in python2.7 when requesting an endpoint, wsgi returns:

prometheus_1  | Traceback (most recent call last):
prometheus_1  |   File "/usr/local/lib/python2.7/wsgiref/handlers.py", line 85, in run
prometheus_1  |     self.result = application(self.environ, self.start_response)
prometheus_1  |   File "/usr/local/lib/python2.7/site-packages/prometheus_client/exposition.py", line 125, in prometheus_app
prometheus_1  |     start_response(status, [header])
prometheus_1  |   File "/usr/local/lib/python2.7/wsgiref/handlers.py", line 173, in start_response
prometheus_1  |     assert type(status) is StringType,"Status must be a string"
prometheus_1  | AssertionError: Status must be a string

This is reproduced here: https://github.com/Geethree/prometheus-py27-string-bug

I'm not really sure of:

  • If this is really the best way to get around this issue, happy to chat about it!
  • The best way to construct a test to catch the AssertionError in the Daemon thread, happy to modify if someone can toss me a few tips I'd be happy to write some tests =)

Py2 expects this to be encoded while py3 expects this to be a string.

Signed-off-by: Gerald H Spencer <ger.spencer3@gmail.com>
@Geethree

Geethree commented Jul 1, 2021

Copy link
Copy Markdown
Author

@csmarchbanks @Travis-Sun =)

@csmarchbanks csmarchbanks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix! One comment, I am wondering if headers should also be cast as str, which is not happening on 119.

Would you be willing to add a test for this?

status, header, output = _bake_output(registry, accept_header, params)
# Return output
start_response(status, [header])
start_response(str(status), [header])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if lines 118 and 119 should just be cast as str similar to what the end of _bake_output does? Do you only see this issue for requests going to /favicon.ico or other locations as well?

@csmarchbanks

csmarchbanks commented Oct 29, 2021 •

Copy link
Copy Markdown
Member

I ended up fixing this in a slightly different spot with #715, as well as casting the headers as strings which were also causing an exception. Thank you very much for the reproduction and start of a fix though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants