New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 805982 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

On Tricium Progress request, Tricium returns InvalidArgument for very new issues

Project Member Reported by qyears...@chromium.org, Jan 25 2018

Issue description

Background: Tricium polls Gerrit to keep track of new changes; for the period of time in between issue creation and poll completion, there is no run for the Gerrit change, and no GerritChangeToRunID in datastore.

When a Progress request is made during this (usually very brief) time span, Tricium responds with error code 3 (InvalidArgument) with the message "failed to find run ID for Gerrit change".

This causes the tricium plugin to log an error, even though this is expected behavior sometimes.

Proposal:

 (1) We could make Tricium respond with success plus an empty ProgressResponse, i.e. ProgressResponse{RunId: 0, State: PENDING, FunctionProgress: nil}, whenever a request is made for a Gerrit change that has no runs.

 (2) We could also keep the current behavior, perhaps changing the plugin error log to a warning log.

Any opinions about what makes more sense in this case?
 
Status: Started (was: Assigned)
Summary: On Tricium Progress request, Tricium returns InvalidArgument for very new issues (was: On Tricium Progress request, Tricium could return InvalidArgument for very new issues)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/15fb9c61abb3b377e4bb2f25e769824cb62ad234

commit 15fb9c61abb3b377e4bb2f25e769824cb62ad234
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Wed Feb 21 00:10:21 2018

Change Progress to return NotFound status if run not found

Reason: Asking for a Gerrit change that doesn't yet have a
run associated with it would be more accurately classified
as "NotFound" rather than "InvalidArgument".

This is expected to happen sometimes.

Some context in https://crrev.com/c/907374.

Bug:  805982 
Change-Id: Ie82788da435a78ce9242a12edb64e879d32be906
Reviewed-on: https://chromium-review.googlesource.com/907594
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/15fb9c61abb3b377e4bb2f25e769824cb62ad234/go/src/infra/tricium/appengine/frontend/rpc_progress_test.go
[modify] https://crrev.com/15fb9c61abb3b377e4bb2f25e769824cb62ad234/go/src/infra/tricium/appengine/frontend/rpc_progress.go

Status: Fixed (was: Started)
Should be fixed with the next deploy.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/gerrit-plugins/tricium/+/c1fdef224af98d97d1f6a9e3098f1a4e26d53029

commit c1fdef224af98d97d1f6a9e3098f1a4e26d53029
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Fri Mar 02 23:21:59 2018

When Progress returns Not Found, just log a warning.

This also reworks the logic and tests for tricium-client so that:

 - _rpcRequest returns a Promise that resolves to a Response
   rather than an object or error message. This allows the
   getProgress function to handle the 404 error case differently,
   while reportNotUseful will still reject with an error for any
   non-OK response.

 - _handleRpcResponse is removed, and several small private helper
   functions are added.

 - Tests are simplified slightly to omit unnecessary then/catch
   clauses. If the promise under test unexpectedly resolves or rejects,
   then the test method will time out and fail.

Bug:  805982 
Change-Id: I510ffd9ab71e1b8197dab59269f0a05b883e216a
Reviewed-on: https://chromium-review.googlesource.com/944518
Reviewed-by: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/c1fdef224af98d97d1f6a9e3098f1a4e26d53029/src/main/resources/static/tricium-client.js
[modify] https://crrev.com/c1fdef224af98d97d1f6a9e3098f1a4e26d53029/test/tricium-client_test.html

Sign in to add a comment