Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Error handling for articles #1504

Closed
hyperreality opened this issue Sep 13, 2016 · 2 comments
Closed

Error handling for articles #1504

hyperreality opened this issue Sep 13, 2016 · 2 comments
Assignees
Milestone

Comments

@hyperreality
Copy link
Contributor

hyperreality commented Sep 13, 2016

Invalid articles currently go to a blank page, rather than the correct error page. The 400 or 404 errors are sent from the server, they're just not dealt with on the client-side. I don't have the time to fix it up and test it now but I think the below is a start, in articles.client.routes.js (or perhaps it should go into the controller although I don't think the controller is ever reached if there's an error). Paging @mleanos

function getArticle($stateParams, ArticlesService) {
  return ArticlesService.get({
    articleId: $stateParams.articleId
  }).$promise.then(function(data) {
    return data;
  }, function(error) {
    console.log(error);
  }) 
}
@mleanos
Copy link
Member

mleanos commented Sep 15, 2016

We're already returning a 404 from the server when an Article isn't found. So we just need to add a 404 handler here to send the user to a 404 view. So this should only involve the client-side addition of $http interceptors for 400 & 404.

I don't know what happened to this PR: #1052. But it looks like it will solve this issue.

@mleanos mleanos added this to the 0.5.0 milestone Sep 15, 2016
@mleanos mleanos self-assigned this Sep 15, 2016
@hyperreality
Copy link
Contributor Author

hyperreality commented Sep 15, 2016

Indeed, that appears to do the trick. However, that approach does have a couple of issues.

  1. The error message is generic, as opposed to preserving the "Article not found" error message sent from the server.
  2. The back button breaks - the previous page just sends you directly back to the error state.

I will look into those issues and attempt to improve the PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants