Bug 337198 - TransferJob have an empty responsecode if the job is fails
Summary: TransferJob have an empty responsecode if the job is fails
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Unmaintained
Component: general (show other bugs)
Version: 4.13.2
Platform: Debian unstable Linux
: NOR normal
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-07 17:42 UTC by Sandro Knauß
Modified: 2014-10-10 02:11 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.14.2
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sandro Knauß 2014-07-07 17:42:31 UTC
if I create a TransferJob and want to handle the http responsecode, I'll get an empty responsecode it the job fails. See code snippset:

start(const KUrl &url) {
    QMap< QString, QVariant > map;
    map[QLatin1String("errorPage")] = false;
    
    KIO::TransferJob* job = KIO::get( url, KIO::NoReload, KIO::HideProgressInfo );
    job->setMetaData( map );
    connect(job, SIGNAL(result(KJob*)), this, SLOT(slotResult(KJob*)) );
}

slotResult(KJob* job)
{
   KIO::TransferJob *tjob = qobject_cast<KIO::TransferJob *>(job);
   if ( job->error() ) {
       kDebug() << "Fetching failed" << tjob->queryMetaData(QLatin1String("responsecode"));
   } else {
       kDebug() << "all good:)" <<  tjob->queryMetaData(QLatin1String("responsecode"));
   }
}

Reproducible: Always

Actual Results:  
run code and see if the webpage fails with 404 or 500:
"Fetching failed"  ""
and with all went ok:
"all good:)" "200"

you can also set
   map[QLatin1String("errorPage")] = true;

than you get:
for 500:
"Fetching failed"  ""

for 404:
"all  good:)" "404"

for 200:
"all  good:)" "200"

Expected Results:  
also set the responsecode if the job is failing:
"Fetching failed"  "404" / "Fetching failed"  "500"

I think the problem, is that the error is emitted before the responsecode is set to the metadata:
see
bool HTTPProtocol::proceedUntilResponseHeader()
{
  [...]
  while(true) {
       [...]
       //responsecode is used now to deside if authentification is needed, so it is available
       if (!m_isLoadingErrorPage && isAuthenticationRequired(m_request.responseCode)) {
          readBody(true);
      }
      if (m_iError || m_isLoadingErrorPage) {
          return false;
          //i think this should be a break, or set reponsecode before
      } 
  }
  [...]
  //move this line up
  setMetaData(QLatin1String("responsecode"), QString::number(m_request.responseCode));
  return true;
}
Comment 1 Dawit Alemayehu 2014-09-13 11:20:00 UTC
https://git.reviewboard.kde.org/r/120178/
Comment 2 Dawit Alemayehu 2014-09-19 11:25:41 UTC
Git commit 685c8e172e8fea0c8c29781a2c376ce9d994ed87 by Dawit Alemayehu.
Committed on 13/09/2014 at 05:30.
Pushed by adawit into branch 'KDE/4.14'.

Send HTTP response code for error condition as well.
FIXED-IN: 4.14.2
REVIEW: 120178

M  +1    -0    kioslave/http/http.cpp

http://commits.kde.org/kdelibs/685c8e172e8fea0c8c29781a2c376ce9d994ed87
Comment 3 Dawit Alemayehu 2014-10-10 02:11:20 UTC
Git commit 8930b5b5ab8fb6431480fc4d20ba1abca6f91ca1 by Dawit Alemayehu.
Committed on 06/10/2014 at 12:45.
Pushed by adawit into branch 'master'.

frameworks port of commit 685c8e1:

Send HTTP response code for error condition as well.

M  +1    -0    src/ioslaves/http/http.cpp

http://commits.kde.org/kio/8930b5b5ab8fb6431480fc4d20ba1abca6f91ca1