reused api oauth nonce causes oops

do you have any code that would retry requests in there?

There's no code to retry requests in lp:kanban. I don't think there
is any in lazr.restful.

The relevant code is

def get_person_directly_assigned_bugs(launchpad, person):
    """Generator yields L{Bug}s assigned to C{person}.

    @param launchpad: A C{Launchpad} instance.
    @param person: A C{person} instance from Launchpad.
    """
    for bug_task in person.searchTasks(status=RELEVANT_STATUSES,
                                       assignee=person):
        # It's nice to see fixed bugs for the sake of a sense of
        # accomplishment, but we don't want the kanban to get too big.
        trace(bug_task)
        if (bug_task.status == "Fix Released"):
            date_closed = bug_task.date_closed
            age = datetime.now(date_closed.tzinfo) - date_closed
            if (age > timedelta(days=31)):
                trace("fixed too long ago, omitting")
                continue
        yield _create_bug(bug_task)

On the server side, we shouldn't record an OOPS for this; its something clients can trivially cause (via hacking or mistakes).

On 6 April 2011 17:38, Robert Collins <email address hidden> wrote:
> On the server side, we shouldn't record an OOPS for this; its something
> clients can trivially cause (via hacking or mistakes).

Well, that's true, but on the other hand it seems like in this case it
is happening because of a fault in Launchpad, or perhaps lplib. In
that sense it's a bit like other 400-type errors.

On Thu, Apr 7, 2011 at 11:23 AM, Martin Pool <email address hidden> wrote:
> On 6 April 2011 17:38, Robert Collins <email address hidden> wrote:
>> On the server side, we shouldn't record an OOPS for this; its something
>> clients can trivially cause (via hacking or mistakes).
>
> Well, that's true, but on the other hand it seems like in this case it
> is happening because of a fault in Launchpad, or perhaps lplib.  In
> that sense it's a bit like other 400-type errors.

If its a fault in LP, then yes we should fix it. However, we want
OOPSes to always indicate that there is something developers or ops
need to do : its a server /fault/ reporting system. We don't generally
log OOPSes for 400 class errors; 404's with a canonical-site referer
are a key exception, for instance.

I'm not sure if bac took this on with the intention of just silencing
the oops, or of a deeper investigation.

As far as I can see the client is not sending a reused nonce, and
Launchpad is incorrectly saying it has already been reused, which
would be a bug in lp. I think it is at worth doing the investigation
before deciding to squash this class of errors: it is something that
_could_ be generated by a client error but I don't see any evidence it
actually is.

It *can* be caused by bust clients and its a normal situation to have
occur => its *not* appropriate as an OOPS.

Separate to that there is a good question around why you are seeing
it, but the reason this bug is critical is the OOPSness. Lets not
conflate these things: even if we diagnose why you are seeing this and
conclude its a server bug, we'd still have other clients able to
trivially inject terrible noise into our crash database by sending
previously used nonces at LP.

Once the OOPS is removed, this can be either closed and a new one
without the OOPS attached to it opened, or this bug can be dropped to
high-or-low priority.

-Rob

Based on Robert's comments I will be fixing this bug to only avoid the OOPS for improper token/nonce/timestamp client data.

I discussed this offline with Robert.

I have no objection to squashing the oops alone.

If a valid client request causes an error (whether it oopses or not)
that also matches the definition of critical, but we can handle that
as a separate bug. I probably won't bother filing it unless I see it
occur again.

Fixed in stable r13127 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/13127>.