Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-icon: Gracefully handle network errors #2530

Closed
epelc opened this issue Apr 23, 2015 · 10 comments
Closed

md-icon: Gracefully handle network errors #2530

epelc opened this issue Apr 23, 2015 · 10 comments
Assignees
@programmist
Milestone
1.0-rc1

Comments

@epelc
Copy link
Contributor

epelc commented Apr 23, 2015

It would be nice if md-icon gracefully handled network errors. Right now it isn't safe to use md-icon if your endpoint is down. It will throw exceptions because it doesn't check certain objects/properties before it uses them.

For example the following line isn't safe

material/src/components/icon/iconService.js

Line 357 in 977a495

var setName = id.substring(0, id.lastIndexOf(':')) || '$default'; 

"TypeError: id.lastIndexOf is not a function
    at loadFromIconSet (http://dev.*******.com/app/vendor.js:25561:40)
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:112:113
    at n.$get.n.$eval (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:126:15)
    at n.$get.n.$digest (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:123:106)
    at n.$get.n.$apply (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:126:293)
    at l (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:81:240)
    at M (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:85:342)
    at XMLHttpRequest.e (https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js:86:418)"

You can test this by just shutting your server off after you load the first page and go to a second page which has a new icon. It might also be a good idea when this is fixed to implement a missing icon which could be hard coded into the js. This could be used instead if there are any errors loading icons.

Also this hopefully isn't too important on desktop as servers don't go down often(we hope) and you shouldn't be losing your network. But it might be a much larger problem on mobile where your signal constantly goes in and out if your not in a large city.
@ThomasBurleson ThomasBurleson modified the milestone: 0.10.0 Apr 24, 2015
@programmist
Copy link
Contributor

Did this occur when using md-svg-icon or md-svg-src (or both)?

@epelc
Copy link
Contributor Author

epelc commented Apr 28, 2015

@programmist I was using md-svg-icon. I setup my icons using several $mdIconProvider.icon() calls.

@programmist
Copy link
Contributor

@epelc Thanks for the info and the bug report.

@epelc
Copy link
Contributor Author

epelc commented Apr 28, 2015

@programmist No problem let me know if you want me to test anything.

@programmist
Copy link
Contributor

@epelc Could you verify my fix works for you using this branch?
https://github.com/programmist/material/tree/wip-icon-bugs

@neokeats
Copy link

neokeats commented Jun 8, 2015

Hi,
+1

@programmist
Copy link
Contributor

Hi @neokeats. Did my branch fix the issue for you? It seems to be working for me.

@neokeats
Copy link

neokeats commented Jun 8, 2015

Hi,
I build your fork, replaced the bower lib by the dist generated ones, and tested it.
I don't have the execption any more, but angular still throw something in the console since my backend is django based but it seems to be just a console.log output of the response.
The file is not found generate a 404 page.
For information i'm only trying to get the starter es6 version to work, nothing fancy.

"17:58:37:546 - UsersDataservice::loadAll()" angular.js:11658:17
GET 
XHR 
http://127.0.0.1:8000/assets/svg/avatars.svg [HTTP/1.0 404 NOT FOUND 10ms]
"
<!DOCTYPE html>
<html lang="en">
<head>
  <meta http-equiv="content-type" content="text/html; charset=utf-8">
  <title>Page not found at /assets/svg/menu.svg</title>
  <meta name="robots" content="NONE,NOARCHIVE">
  <style type="text/css">
    html * { padding:0; margin:0; }
    body * { padding:10px 20px; }
    body * * { padding:0; }
    body { font:small sans-serif; background:#eee; }
    body>div { border-bottom:1px solid #ddd; }
    h1 { font-weight:normal; margin-bottom:.4em; }
    h1 span { font-size:60%; color:#666; font-weight:normal; }
    table { border:none; border-collapse: collapse; width:100%; }
    td, th { vertical-align:top; padding:2px 3px; }
    th { width:12em; text-align:right; color:#666; padding-right:.5em; }
    #info { background:#f6f6f6; }
    #info ol { margin: 0.5em 4em; }
    #info ol li { font-family: monospace; }
    #summary { background: #ffc; }
    #explanation { background:#eee; border-bottom: 0px none; }
  </style>
</head>
<body>
  <div id="summary">
    <h1>Page not found <span>(404)</span></h1>
    <table class="meta">
      <tr>
        <th>Request Method:</th>
        <td>GET</td>
      </tr>
      <tr>
        <th>Request URL:</th>
        <td>http://127.0.0.1:8000/assets/svg/menu.svg</td>
      </tr>

    </table>
  </div>
  <div id="info">

      <p>
      Using the URLconf defined in <code>project.urls</code>,
      Django tried these URL patterns, in this order:
      </p>
      <ol>

          <li>

                ^$
                [name='home']

          </li>

          <li>

                ^admin/


          </li>

      </ol>
      <p>The current URL, <code>assets/svg/menu.svg</code>, didn't match any of these.</p>

  </div>

  <div id="explanation">
    <p>
      You're seeing this error because you have <code>DEBUG = True</code> in
      your Django settings file. Change that to <code>False</code>, and Django
      will display a standard 404 page.
    </p>
  </div>
</body>
</html>
"

@ThomasBurleson ThomasBurleson modified the milestones: 0.10.0, 0.11.0 Jun 16, 2015
@programmist
Copy link
Contributor

@neokeats Yes, that is a log of the error message returned from the server. But it's not throwing an exception - only logging the error.

When I test I don't see all that output, but I'm not using Django - just a plain HTTP server.

@programmist programmist mentioned this issue Jul 14, 2015
Closed
@ThomasBurleson ThomasBurleson modified the milestones: 0.11.0, REVISIT Jul 31, 2015
@ThomasBurleson ThomasBurleson added the needs: review This PR is waiting on review from the team label Jul 31, 2015
@programmist
Copy link
Contributor

This issue will close when PR #3718 is merged into master.

@programmist programmist modified the milestones: 0.11.0, REVISIT Jul 31, 2015
@programmist programmist removed the needs: review This PR is waiting on review from the team label Jul 31, 2015
kennethcachia pushed a commit to kennethcachia/material that referenced this issue Sep 23, 2015
@programmist @kennethcachia
fix(icon): Log network errors without throwing exceptions.
eac7fd2 
fixes angular#2530

closes angular#3718
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@programmist programmist

Labels
None yet
Projects
None yet
Milestone
1.0-rc1
Development

Successfully merging a pull request may close this issue.

fix(icon): Log network errors without throwing exceptions.
4 participants
@neokeats @ThomasBurleson @programmist @epelc