Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add extra check when loading model on crud #163

Merged
merged 1 commit into from
May 1, 2012
Merged

Conversation

daraosn
Copy link
Contributor

@daraosn daraosn commented Apr 29, 2012

I found a bug when trying to delete a non-existant record (the application crashes). I added an extra check to avoid this behavior on crud, but the app may still crush, this must be handled within the core.

To reproduce the error:

  1. create a railway app
  2. $ railway g post title description
  3. run app
  4. Go to http://localhost:3000/posts/new and create one post
  5. When redirected in the Post index, open another tab in your browser and go to the same address (http://localhost:3000/posts).
  6. On one tab, delete the record, you will be redirected to the index, and see no records, now go to the other tab (where you still can delete the record), click on delete, and you will find the error.

@daraosn
Copy link
Contributor Author

daraosn commented Apr 29, 2012

This is the error I get:

node.js:201
throw e; // process.nextTick error, or 'error' event on first tick
^
TypeError: Cannot call method 'destroy' of null
at Object.destroy (/Users/user/railwayapp/app/controllers/posts_controller.js:67:17)
at Object. (/Users/user/railwayapp/node_modules/railway/lib/controller.js:372:20)
at Array.0 (/Users/user/railwayapp/node_modules/railway/lib/controller.js:349:24)
at EventEmitter._tickCallback (node.js:192:40)

1602 added a commit that referenced this pull request May 1, 2012
add extra check when loading model on crud
@1602 1602 merged commit 5e71aa4 into 1602:master May 1, 2012
@1602
Copy link
Owner

1602 commented May 1, 2012

Thanks man!

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

Successfully merging this pull request may close these issues.

2 participants