[SYNTH-16456] Postpone reporting results on 404

[Drarig29]

What and why?

There is sometimes some latency between batches and the poll results endpoint. When this happens, the poll results endpoint may return a 404 error because some queried results cannot be found, so the request ends up returning nothing.

Currently, we retry 3 times on 404 errors for that endpoint, and if the request never succeeds, datadog-ci quits and the CI fails.

How?

• Extend our concept of incomplete results to also take care of results that have a 404: the reporting is postponed, then the incomplete results are fetched in next polling cycles.
• We backup polling results in case the last polling cycle encounters a 404, because this last polling cycle has to fetch all results and we'll probably have fetched and reported most results already.
• Like before, incomplete results may still be incomplete at the end of the batch: in that case, the result is reported without a server result and the following log is printed: The information for result {resultId} of test {publicId} was incomplete at the end of the batch.

Review checklist

• Feature or bugfix MUST have appropriate tests (unit, integration)

[etnbrd]

If we don't need this, could we rather make it optional?

[Drarig29]

I'm afraid of how it could break customer's code. I'll check internally (web-ui and CI integrations)

[etnbrd]

Given the similarity between this function and the return statement from the above, could we merge the two, and add only the difference if pollResult is not available?

const resultFromBatch = {
    executionRule: resultInBatch.execution_rule,
    initialResultId: resultInBatch.initial_result_id,
    isNonFinal: isNonFinalResult(resultInBatch),
    location: getLocation(resultInBatch.location, test),
    passed: hasResultPassed(resultInBatch, isUnhealthy, hasTimedOut, options),
    resultId: getResultIdOrLinkedResultId(resultInBatch),
    retries: resultInBatch.retries || 0,
    maxRetries: resultInBatch.max_retries || 0,
    selectiveRerun: resultInBatch.selective_rerun,
    timedOut: hasTimedOut,
}

if (pollResult) {
  return {
    ...resultFromBatch,
    result: {
      ...pollResult.result,
      ...(safeDeadlineReached ? {
        failure: new BatchTimeoutRunawayError().toJson()
        passed: false
        } : timedOutRetry || hasTimedOut ? {
          failure: new {code: 'TIMEOUT', message: 'The batch timed out before receiving the result.'}
          passed: false
        } : {})
    },
    test: deepExtend({}, test, pollResult),
    timestamp: pollResult.timestamp
  }
} else {
  return {
    ...resultFromBatch,
    test: deepExtend({}, test),
    timestamp: Date.now()
  }
}

This code might not be the best solution, but will hopefully give you an idea.

[teodor2312]

This way would make it more concise, but i'm not sure if having the two functions is not actually clearer and easier to understand 🤔

[Drarig29]

I prefer the 2 functions as well tbh 😁

[teodor2312]

Apart from the question the Etienne has raised LGTM 👍