My work on POE::Filter::SSL is relevant here. I needed a way to pass OOB data back to the readwrite wheel during the SSL negotiation. The method LotR and I came up works great. I pass an array ref to get_one (lets call it $oob), and any time a filter has OOB data, it blesses it into a namespace, and pushes it onto @$oob. For Filter::SSL, I used the OOB namespace 'POE::OOB::put' for data that needs to be directly sent out the wheel's driver. Filter::Stackable and Wheel::ReadWrite needs to be aware of this OOB data, so I've patched them too. We can use this method to add 'POE::OOB::error' and handle it by sending an event.
Relevant files: http://svn.xantus.org/sprocket/trunk/Sprocket/lib/POE/Filter/Stackable.pm http://svn.xantus.org/sprocket/trunk/Sprocket/lib/POE/Filter/SSL.pm http://svn.xantus.org/sprocket/trunk/Sprocket/lib/POE/Wheel/ReadWrite.pm Cheers, David On 8/14/07, Martijn van Beers <[EMAIL PROTECTED]> wrote: > > There was a discussion about this on #poe, which I thought I'd share: > > <+nperez> an isa check every single time something goes through the > filter just seems like overkill... > <+LotR> compared to the work that Filter::XML does? > <+nperez> yes, because it will add up. hold on, writing a benchmark > script > <+LotR> I'm sure it will prove to be nothing compared to parsing an xml > subtree > <+nperez> but what about other filters? Stream? Line? > <+LotR> stream doesn't ever need to generate any errors. nor does line > <+LotR> if you just use those, you can forget about error checking > altogether > * LotR will send the discussion to the list when we're done > <+nopaste> "nperez" at 12.5.72.252 pasted "undef vs isa" (30 lines) at > http://nopaste.snit.ch:8001/10886 > <+nperez> take a look > <+nperez> that's a huge difference > <+LotR> but an isa() still is pretty damn fast. > <+nperez> besides, it is less of a rewrite checking for undef. > consider: > you have to now check ref and then isa on every return value > from the filter > <+nperez> what if it isn't a ref that gets returned? > <+nperez> and isa only worked on blessed objects > <+LotR> you're still going to have to check for undef too > <+nperez> works > <+nperez> exactly, so why do the other checks and you can just do the > one > <+LotR> and it is going to be more of a change if you then have to ask > the filter for extra data > <+nperez> not really, implementing an get_error() isn't as profound of > a > change > <+nperez> it is OOB > <+nperez> without you still have checking for undef for errors in a lot > of modules anyhow > <+nperez> if they want to know more info, they then call get_error() > <+nperez> otherwise, they might not care > <+LotR> that "whout you still.." sentence doesn't parse for me > <+LotR> without. duh > <+nperez> sorry, without get_error(), you still have checking for undef > <+LotR> still don't get it. how does checking for undef for errors in > 'a > lot of modules' have anything to do with filters? > <+nperez> because right now, the de facto indicator for error is > returning undef > <+LotR> no it isn't > <+LotR> in Filter::HTTPD, an HTTP::Response is returned > <+LotR> in Filter::XML, you get a callback > <+LotR> in Filter::SAXBuilder, you get an Error::Simple object > <+LotR> and I don't know any other filters that return errors right now > <+nperez> okay, thinking a little more general, perl built-ins return > undef on errors and set $! > <+nperez> following that convention can't be /that/ bad > <+LotR> right, but is it guaranteed that you always get the chance to > check the error before an extra error is created (meaning > filters > have to store more than one, and the $! analogy breaks down) > <@Leolo> test it against Scalar::Util::blessed > <+nperez> Leolo: that is a possibility too > <+LotR> Leolo: hmm? > <@Leolo> also, the filter could return undef(), then you ask the filter > ->is_error() or ->error_str() or whatever > <@Leolo> call this wrapped in POE::Filter::Fallible > <+LotR> I just don't particulary like the extra API > <@Leolo> s/call/all/ > <+nperez> well, it's incomplete as is. Error states are simply > unaccounted for. Most of the modules that I have been looking > through in the past 10 minutes make all sorts of assumptions > on > input > <+nperez> if there is an error, it will just die > <+nperez> or return [ ] > <+nperez> in which case, how do you know there was an error? > <+LotR> you don't. But I like the simplicity of array ref in; array ref > out > <@Leolo> they probably assume they are talking to another > POE::Filter::Foo. in which case there should be no error > <@Leolo> lotr : true > <+nperez> the simplicity fails. If the filter gets into an inconsistent > state due to bad input, but never tells the outside world > what > is going on, that's just poor > <+nperez> returning [] is just too overloaded > <+LotR> plus, if you stack the filters, can you make sure you will just > have had one error if you get out of the filter? > <+LotR> nperez: I'm not saying returning [] on error is a good idea, it > isn't. > <+nperez> there probably needs to be a way to aggregate/bubble errors > up > from stacked filters > <+LotR> I'm saying returning an error object in the array ref out keeps > the simplicity > < imMute> maybe it should return some kind of POE::Filter::Error object > on error > < imMute> and then the wheel thats processing it can deal with the error > < imMute> or the next filter in stackable can just pass it on > <+nperez> LotR: but that adds checking in-band which is a performance > killer > <+nperez> maybe no killer, but detrimental to performance i should say > <+nperez> maybe no killer, but detrimental to performance i should say > < imMute> its better than just suddenly not getting any records from a > filter and not knowing why > <+LotR> nperez: right. so we disagree on how bad such a performance hit > that is > <+nperez> imMute: getting an undef should be a strong idicator of error > in which case you check the error property on the filter > <+nperez> agreed on that point LotR > < imMute> nperez: are we going to standardize the name of that call, > and > what it should return? > * LotR wonders if there are no filters that return undef for reasons > other than error > <+nperez> imMute: that's what we would like to happen, yes > < imMute> personally I think we should standardize the arrayref in, > arrayref out first, because I know there are filters that > dont > follow that > <+nperez> it is standard, if they subclass POE::Filter > <+nperez> and stay "true" to form > < imMute> some dont though > <+nperez> then we should file bugs on rt for them :) > < imMute> i know few of BinGOs' filters subclass POE::Filter > < imMute> and he complained loudly when i added an isa() check to > Stackable > <@BinGOs> all of them do. > <+nperez> I did too > <+nperez> I had to make changes to make the filter stabled > <+nperez> stackable > <+purl> well, stackable is newer than PFX? > <@BinGOs> this is why I was screaming blue fucking murder earlier. > < imMute> maybe its changed since i last messed with filters, its been > a > few months > <+nperez> It is implicit that filters take aref in and spit aref out > <+LotR> explicit in the docs actually > <+nperez> right > <@BinGOs> My take is POE::Filter shouldn't shit out anything they > shouldn't > <+LotR> BinGOs: meaning? > <+purl> somebody said meaning was in the individual, whether a > definition > <@BinGOs> otherwise a world of fucking hurt is going to happen. > <@BinGOs> meaning, don't return anything on a fucking error. > <+LotR> why not? > <@BinGOs> because I will hunt you down like a dog > <+nperez> BinGOs: if there is an error, I'd like to know about it > <@BinGOs> then 'warn' > <+nperez> returning [] is not tenable > <+nperez> the state of the filter may need to be reset > <+nperez> I can't capture warns and adjust accordingly > <@BinGOs> if you are in a POE::Filter::Stackable then everyone may not > be > following the rules. > <+nperez> which is why we are wanting to update the POE::Filter API and > introduce a standard way to report errors > <+LotR> BinGOs: there'll be new rules :) > <+nperez> heh > <@BinGOs> I am not the bad guy here >:) > <+nperez> BinGOs: if you want patches to your modules after the change, > I'll help, promise. Because you have a shit load and I don't > blame you for beign resistant to change ;) > <+LotR> BinGOs: oh no? so what happens if Filter::IRC gets a line that > doesn't parse? > <@BinGOs> it warns > <+LotR> which isn't very nice > <@BinGOs> and b). I am not using POE::Filter::IRC these days. > <+LotR> Filter::IRCD then > <@BinGOs> and c). yes, POE::Filter::IRCD warns > <+nperez> warnings disappear into the aether and don't control > autocorrecting procedures > <+LotR> BinGOs: which you can't handle very nicely. > <@BinGOs> Okay, you have a POE::Filter::Stackable > < imMute> what does PCI use for the client then? > <@BinGOs> the first filter produces an error, what happens next ? > <@BinGOs> ( you have four filters in the chain ) > <+nperez> stackable checks the error property, sets its own error > property and returns undef > <+nperez> skipping the other three > <@BinGOs> do you produce an event at the end ? > <+LotR> or > <+LotR> it sticks an error object in the listref out. (and probably > skips > the other three filters) > < imMute> BinGOs: no > <+LotR> BinGOs: filters don't know anything about POE events. I want to > keep it that way > <@BinGOs> PCI uses Filter::IRCD and POE::Filter::IRC::Compat in a > stackable. > < imMute> BinGOs: stackable doesnt produce output unless the last > filter > returns something > <@BinGOs> SO what happens to your error then ? > <+nperez> returning undef should be an indicator to check the error > property on the filter > <+LotR> BinGOs: read the scrollback? we've been over this and haven't > come to an agreement. nperez and I have different views on how > to > do it > <+LotR> but we agree there should be some error reporting in filters > <@BinGOs> Hell, I'll work around whatever you come up with anyways >:) > <+nperez> haha > <@BinGOs> difficult audience. > <+nperez> Ultimately is up to dngor about an API change > <+nperez> we are wanting to do something rather fundamental > <+nperez> especially with a 1.0 approaching > <@BinGOs> this is why I am stomping my little feet. > <+nperez> If it's broken, it's broken. Bitching how much the fix hurts > isn't going to make it go away :) > <+LotR> another reason not to do an ->get_error() method. no need to > change old filters who don't care about error reporting > <+nperez> if they don't care about error reporting, they aren't > checking > their inputs enough ;) > <@BinGOs> I am a team player and have a 'Can do attitude' > <@BinGOs> the whole POE::Filter sub-class thing was no problem to me. > <+nperez> BinGOs: we aren't interviewing you here. You are more than > welcome to be the devil's advocate and general asshole. It > helps :) > <@BinGOs> Yes, I am playing devil's advocate > <@BinGOs> and yes, I am an asshole. > <+nperez> You represent a large swath of incumbent modules. Your > opinion > counts > <@BinGOs> I always have an eye on backwards compatibility. > <@perigrin> to watch it slip away > <+nperez> That is certainly something to consider. LotR's idea is > probably better for backwards compat > <@Leolo> backwards compatible with "do nothing useful" is easy > > > -- David Davis Software Engineer
