Uploaded image for project: 'Qt Creator'
  1. Qt Creator
  2. QTCREATORBUG-33493

Give the dashboard server more freedom in what statuscodes to use

XMLWordPrintable

    • Icon: Suggestion Suggestion
    • Resolution: Unresolved
    • Icon: Not Evaluated Not Evaluated
    • None
    • None
    • Axivion
    • None

      The low-level code that looks at server responses is currently very strict when looking at HTTP response codes.

      HTTP status codes and ErrorDto::type both are similar in nature, but of course not every application level error can be expressed with HTTP status codes. The idea is thus, that applications working with the Dashboard API should mainly rely on ErrorDto::type.

      In principle it should be sufficient to use HTTP status codes to determine the nature of a response, i.e. whether it was successful or not and thus whether to expect the regular JSON or the error JSON in the body.

      See below an excerpt of the current code which would fail to work, if e.g. we'd change the statuscode coming with "InvalidFilterException" to be 500 (which likely won't ever happen but it shouldn't really make a difference for the client.)

      if (doneWith == DoneWith::Success && statusCode == httpStatusCodeOk
          && contentType == s_jsonContentType) {
          *storage = reply->readAll(); dtoStorage->url = reply->url();
          return DoneResult::Success;
      }
      
      QString errorString;
      
      if (contentType == s_jsonContentType) {
          const Result<Dto::ErrorDto> error = Dto::ErrorDto::deserializeExpected(reply->readAll());
          if (error) {
              if constexpr (std::is_same_v<DtoType, Dto::DashboardInfoDto>) {
                  // Suppress logging error on unauthorized dashboard fetch
                  if (!dtoStorage->credential && error->type == "UnauthenticatedException")
                  {
                      dtoStorage->url = reply->url(); 
                      return DoneResult::Success; 
                  }
                  
              }
              if (statusCode == 400 && error->type == "InvalidFilterException"
                  && !error->message.isEmpty())
              {
                  // handle error.. showFilterException(error->message);
                  // return DoneResult::Error; 
              }
      

      In other words, for the error cases (where an ErrorDto can successfully parsed) there is not really a good reason to be stricter than maybe expecting just any 4xx/5xx code or alternatively not a 2xx code (assuming that 1xx and 3xx cannot happen as they are usually handled by the HTTP lib already).

      Note, that in the past it has happened that we were unhappy with the concrete status code sent by the server. Changing that though is not easy if it breaks existing clients.

        For Gerrit Dashboard: QTCREATORBUG-33493
        # Subject Branch Project Status CR V

            cstenger Christian Stenger
            daniel_hofmann Daniel Hofmann
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:

                There is 1 open Gerrit change