| < Previous by Date | Date Index | Next by Date > |
| < Previous in Thread | Thread Index | Next in Thread > |
The major use-case we need to fix is this: InviteSession based MESSAGE requests where the far end returns 415 (mime-type invalid) for some types (text/plain works, but text/html doesn't, application/im-iscomposing+xml doesn't). Without the Contents* of the request that failed, we can't tell which type failed to determine if we need to notify the user sending the message. Scott: You have an opinion on if changing the callback type is an ok way to proceed? Francis: I have attached a prototype patch for this, it has only been compile tested if you want to try it and flesh it out some more. -Aron On Wed, Dec 8, 2010 at 12:39 PM, Francis Joanis <francis.joanis@xxxxxxxxx> wrote: > Hi, > I like what you are proposing since any user-specific information would go > in the user-derived Contents object: it acts a bit like an AppDialog/Set in > that sense. > We could even implement this by only caching the Contents of the last sent > NIT request, since they're all sent serially (so no need to do the extra > management on mNITQueue). This would mainly require adding a new member > variable (probably an auto_ptr<Contents>) to InviteSession and changing the > various on* callbacks in InviteSessionHandler. > In the case of the REFER (which is also using mNITQueue), it would most > likely have a NULL Contents, so it could get ignored on the callbacks (i.e. > no need to modify the onRefer* callbacks). > I started playing around with the code but I'd like to know if such a change > would be a good one or not from the perspective of the more experienced > developers. Changing callback APIs seems like a somewhat major change (i.e. > it will break current apps) and I wouldn't want to do it unless there is > really a good use case. > Regards, > Francis > > > On Wed, Dec 8, 2010 at 1:32 PM, Aron Rosenberg <arosenberg@xxxxxxxxxxxx> > wrote: >> >> For a slightly different issue (matching 415's to outbound MESSAGE >> requests) I was looking at modifying InviteSession to remove the >> SipMessage* from the mNITQueue upon end of transaction rather than >> beginning of transaction. We could then pass up the Contents* like is >> done with ClientPagerMessage. >> >> onMessageSuccess(resip::InviteSessionHandle ish, const >> resip::SipMessage&status) would become >> onMessageSuccess(resip::InviteSessionHandle ish, const >> resip::SipMessage&status, std::auto_ptr<Contents> contents) >> onMessageFailure(resip::InviteSessionHandle ish, const >> resip::SipMessage &status) would become >> onMessageFailure(resip::InviteSessionHandle ish, const >> resip::SipMessage &status, std::auto_ptr<Contents> contents) >> >> This would also work in the INFO cases with the same changes to add >> std::auto_ptr<Contents> contents to the callbacks. >> >> I also attempted to try the other way, dig into the >> TransactionController / TU and create a "SipMessage* >> findLastRequestForResponse(SipMessage*)" that used the tid, but this >> would require adding a number of public functions to SipStack, >> TransactionMap and some other classes. >> >> -Aron >> >> Aron Rosenberg >> Logitech Inc. (SightSpeed Group) >> >> >> >> On Wed, Dec 8, 2010 at 10:08 AM, Francis Joanis >> <francis.joanis@xxxxxxxxx> wrote: >> > So I guess it leaves me with the original workaround of leveraging the >> > fact >> > that all NITs are sequential. Also I think I could use an outgoing >> > DumFeature to inspect outgoing messages, but that would be called for >> > all >> > outgoing messages, which will impact performance. >> > Thanks, >> > Francis >> > >> > On Wed, Dec 8, 2010 at 11:28 AM, Scott Godin <sgodin@xxxxxxxxxxxxxxx> >> > wrote: >> >> >> >> The branch parameter is "reset" in DialogUsageManager send (ln 824) - >> >> so >> >> it won't help to store the branch parameter in the message created >> >> (same >> >> applies for onReadyToSend). >> >> Scott >> >> >> >> On Wed, Dec 8, 2010 at 11:08 AM, Francis Joanis >> >> <francis.joanis@xxxxxxxxx> >> >> wrote: >> >>> >> >>> Hi Scott, >> >>> Thanks for your reply. >> >>> What about if we changed InviteSession::info/refer/message to return a >> >>> _copy_ of the constructed SipMessage rather than returning void? >> >>> I ran a quick test and it looks like the transaction id is set when >> >>> the >> >>> message is constructed. That way, I can immediately cache the SIP >> >>> transaction id to pair it with my "key". This removes the need for my >> >>> extra >> >>> "key list" and does not require message inspection in onReadyToSend. >> >>> (I've also attached a patch to do this.) >> >>> Thanks, >> >>> Francis >> >>> >> >>> On Wed, Dec 8, 2010 at 9:12 AM, Scott Godin <sgodin@xxxxxxxxxxxxxxx> >> >>> wrote: >> >>>> >> >>>> ...inline... >> >>>> >> >>>> On Tue, Dec 7, 2010 at 8:40 PM, Francis Joanis >> >>>> <francis.joanis@xxxxxxxxx> wrote: >> >>>>> >> >>>>> Hi guys, >> >>>>> I have a question about sending multiple NIT messages logically "at >> >>>>> the >> >>>>> same time" (with DUM). I do know that the current code doesn't allow >> >>>>> for >> >>>>> real parallel NIT requests (i.e. the NIT queue in InviteSession) - >> >>>>> but >> >>>>> that's not my issue. >> >>>>> The issue I have is regarding how to match incoming INFO responses >> >>>>> to >> >>>>> their request. Imagine I need to send INFO messages regarding >> >>>>> application-specific transactions (like DB transactions or whatever >> >>>>> application specific thing like a button click). I'll call those >> >>>>> non-SIP >> >>>>> transaction ids "keys". >> >>>>> If I can't have the keys passed into the INFO body and then have >> >>>>> them >> >>>>> resupplied back into the INFO response, there is no easy way of >> >>>>> matching >> >>>>> which response is for which request (especially if my application >> >>>>> would >> >>>>> allow multiple requests to be sent (queued in NIT queue) at the same >> >>>>> time). >> >>>>> The "workaround" would be to leverage the fact that NITs in an >> >>>>> INVITE >> >>>>> dialog are all serialized: I could keep a separate list of "keys". >> >>>>> If I were >> >>>>> to add my key to the list right before sending the INFO message, I >> >>>>> would >> >>>>> then know that the next INFO response will relate to that key >> >>>>> (oldest >> >>>>> element of the key list). But this can get messy since I would need >> >>>>> to make >> >>>>> sure that the "key list" is properly managed. >> >>>> >> >>>> [Scott] This solution works without having to modify DUM, and its >> >>>> likely very similar to how it would be implemented in DUM if we added >> >>>> some >> >>>> kind of key/id to the info() call and response callback. >> >>>> Alternatively, you >> >>>> could track the transaction id's as the INFO messages flow out of the >> >>>> invite >> >>>> session by watching the onReadyToSend callback, but I don't think >> >>>> that >> >>>> offers any advantage over assuming the requests are serialized. >> >>>> >> >>>>> >> >>>>> If I look at the similar scenario but with INVITE messages, one can >> >>>>> easily leverage the AppDialogSet/... to do this: set the "key" on >> >>>>> the >> >>>>> AppDialogSet and then it will be easily accessible once INVITE >> >>>>> responses >> >>>>> arrive (extracted from the INVITE session). >> >>>>> I was thinking of extracting the id of the SIP transaction created >> >>>>> for >> >>>>> the INFO to use it for a lookup between the SIP transaction id and >> >>>>> my "key". >> >>>>> That way, when handling onInfo* callbacks I would be able to access >> >>>>> the SIP >> >>>>> transaction id from the response message and map it with my "key". >> >>>>> However, this might not be feasible if the SIP transaction id is >> >>>>> unknown to the DUM at this time (prior to sending it). >> >>>>> What do you make out of this? >> >>>>> I suspect this will get worst if reSIProcate ever allows multiple >> >>>>> NITs >> >>>>> in parallel at the same time (in an INVITE dialog), since then there >> >>>>> would >> >>>>> really be no easy way of matching them up to application specific >> >>>>> data (or >> >>>>> am I asking for an AppTransaction class ;)?) >> >>>> >> >>>> [Scott] Not sure we would ever allow this - I remember this being >> >>>> discouraged by the IETF. >> >>>> >> >>>>> >> >>>>> Thanks a lot, >> >>>>> Francis >> >>>>> _______________________________________________ >> >>>>> resiprocate-devel mailing list >> >>>>> resiprocate-devel@xxxxxxxxxxxxxxx >> >>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel >> >>>> >> >>> >> >> >> > >> > >> > _______________________________________________ >> > resiprocate-devel mailing list >> > resiprocate-devel@xxxxxxxxxxxxxxx >> > https://list.resiprocate.org/mailman/listinfo/resiprocate-devel >> > > >
Attachment:
reworked_nit_for_contents.diff
Description: Binary data