error handing conflicts with wsgiref.validate

[kiilerix]

backlash 0.2.0 tbtools has

    def log(self, logfile=None):
        """Log the ASCII traceback into a file object."""
        if logfile is None:
            logfile = sys.stderr
        tb = self.plaintext.rstrip() + '\n'

        file_mode = getattr(logfile, 'mode', None)
        if file_mode is not None:
            if 'b' in file_mode:
                tb = tb.encode('utf-8')
            logfile.write(tb)

but wsgiref.validate wraps wsgi.errors with something that doesn't have mode ... and thus nothing is written.

I guess logfile.write(tb) should be dedented to get less strict duck typing.

[amol-]

Seems that was put in place exactly to avoid writing when you are testing. See https://github.com/TurboGears/backlash/commit/6a3f5ca32bfcf0f3b822a5d9cd2ed3275ada8fec

Sadly I don't recall anymore what exact problem it was trying to solve as it's like the 2nd commit ever done to backlash. So I'll have to experiment locally to understand what was the goal.

[kiilerix]

Interesting.

It seems to me like it would be a good solution to backout that changeset.

I would say that the right way to avoid writing when testing must be to have the test pass a dummy wsgi.errors . The middleware shouldn't work around that. Especially not hardcoded and quiet and controlled by something as coarse and accidental as modes.

[amol-]

Ok, found what was the purpose of that block of code. When using WebTest, anything that gets written to wsgi.errors causes a crash because WebTest takes for granted that if anything was written there it means a fatal error. So backlash writes nothing if something different from a real file is provided. This way it still writes errors to log files, but doesn't get in the way of webtest.

I think this behaviour is a bit messy. I don't like WebTest making assumptions about the content of wsgi.errors and I don't like backlash only writing if the output is a real file.

Our of curiosity, isn't setting a DEBUG handler for backlash logger a doable work around for your situation? Backlash should also log there any kind of crash.

[kiilerix]

If that is how WebTest works, then I would say that anybody writing tests that write to wsgi.errors should mock their code or backlash to catch that text and verify it. Backlash could perhaps have some support for easy mocking of this ... but I don't think it should have to.

And agreed, with the tradition of duck typing in Python, it is unfortunate to hardcode special handling for "real file". But also, "non real files" could easily happen to implement the mode attribute that would make this detection fail.

Yes, debug logging for backlash would show the lost message. But for me, it took a while to consider that backlash where to "blame" for the missing error.

[amol-]

I made the change and addressed TG test suite to still pass with the new backlash. I verified the changes also on a newly quickstarted app both when running the app with "gearbox server" and when running tests with "nosetests -v". In both cases it behaved as I expected.

I think it would benefit from more testing on real world web apps that might be opening anything as wsgi.errors. Probably making a prerelease of backlash might allow people to install it if they want to try it.

[kiilerix]

Thanks