Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Show error in PackageDetailView when network is unavailable and package is not in cache #666

Merged

Conversation

izuzak
Copy link
Contributor

@izuzak izuzak commented Sep 25, 2015

This fixes #634. It seems that AtomIoClient.fetchFromCache calls the callback with a special combination of values in case the network is unavailable and the package was not found in cache. This special value was handled elsewhere, but needed to be handled in PackageDetailView as well.

068ce24 adds a failing spec, which triggers this error:

PackageDetailView
  it shows an error when package metadata cannot be loaded from the cache and the network is unavailable
    TypeError: Cannot read property 'on' of undefined
      at PackageCard.module.exports.PackageCard.handleButtonEvents (/Users/izuzak/github/settings-view/lib/package-card.coffee:157:17)
      at PackageCard.module.exports.PackageCard.initialize (/Users/izuzak/github/settings-view/lib/package-card.coffee:69:6)
      at PackageCard.View (/Users/izuzak/github/settings-view/node_modules/atom-space-pen-views/node_modules/space-pen/lib/space-pen.js:184:25)
      at new PackageCard (/Users/izuzak/github/settings-view/lib/package-card.coffee:11:3)
      at PackageDetailView.module.exports.PackageDetailView.completeInitialzation (/Users/izuzak/github/settings-view/lib/package-detail-view.coffee:63:26)
      at /Users/izuzak/github/settings-view/lib/package-detail-view.coffee:98:10
      at /Users/izuzak/github/settings-view/lib/atom-io-client.coffee:32:9
      at AtomIoClient.<anonymous> (/Users/izuzak/github/settings-view/spec/package-detail-view-spec.coffee:69:7)
      at AtomIoClient.module.exports.AtomIoClient.package (/Users/izuzak/github/settings-view/lib/atom-io-client.coffee:30:6)
      at PackageDetailView.module.exports.PackageDetailView.fetchPackage (/Users/izuzak/github/settings-view/lib/package-detail-view.coffee:89:5)
      at PackageDetailView.module.exports.PackageDetailView.loadPackage (/Users/izuzak/github/settings-view/lib/package-detail-view.coffee:83:10)
      at PackageDetailView.module.exports.PackageDetailView.initialize (/Users/izuzak/github/settings-view/lib/package-detail-view.coffee:59:6)
      at PackageDetailView.View (/Users/izuzak/github/settings-view/node_modules/atom-space-pen-views/node_modules/space-pen/lib/space-pen.js:184:25)
      at new PackageDetailView (/Users/izuzak/github/settings-view/lib/package-detail-view.coffee:22:3)
      at [object Object].<anonymous> (/Users/izuzak/github/settings-view/spec/package-detail-view-spec.coffee:71:16)

d8cc61f fixes the problem by checking the data returned by AtomIoClient and verifying that it has at least the package's name property before showing the package card.

cc @kevinsawicki and @thedaniel for 👀

@thedaniel
Copy link
Contributor

AtomIoClient.fetchFromCache calls the callback with a special combination of values in case the network is unavailable and the package was not found in cache.

CLASSIC @thedaniel! Anyway, this seems fine for a quick fix, but if we're going to show an error, maybe we should just pass in an error instead of an imaginary value, and things will work in a more predictable way. Of course, that requires updating the other callers that special case {}

@izuzak
Copy link
Contributor Author

izuzak commented Sep 28, 2015

Anyway, this seems fine for a quick fix, but if we're going to show an error, maybe we should just pass in an error instead of an imaginary value, and things will work in a more predictable way. Of course, that requires updating the other callers that special case {}

Yep, definitely agree with you. I'm intentionally going for a quick fix until I get a better understanding of this package's crazy complexity (I'm fixing small issues here and there for now). I'll open a new issue so that we don't forget to refactor this. 👍

izuzak added a commit that referenced this pull request Sep 28, 2015
…ine-not-cached-not-installed

Show error in PackageDetailView when network is unavailable and package is not in cache
@izuzak izuzak merged commit 946a888 into master Sep 28, 2015
@izuzak izuzak deleted the iz-fix-view-package-details-not-online-not-cached-not-installed branch September 28, 2015 11:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: Cannot read property 'on' of undefined
2 participants