Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

No more crashing on json parsing. #16

Merged
merged 2 commits into from

2 participants

@alexindigo

Added try/catch to JSON.parse.
On error emits 'error' event, which can be handled much nicer than JSON.parse throw.

Lyrics: Some people used to send query string in the body of POST request
and if it crashes the server doesn't look too good.

@mikeal
Owner

remind me, do we have a req error handler in tako that will resp.error() this?

if not we should add one before we put it in.

@alexindigo

For me it throws by default,
but this one solved the problem:

  req.on('error', function(err) {));

If you think it's not enough,
I can add something for default case.

Although, I'd personally prefer

req.on('json', function(err, obj) {});

(Looks more like node.js and easier to handle hiccups)

instead of what we have right now

req.on('json', function(obj) {});

But it's bigger refactor (API changes, etc) and I see the point when sometimes :) people want centralized error handling.

What you think?

@alexindigo

Last one is workaround for Firefox 11,
it sends Content-Type: application/json; charset=UTF-8 instead of just application/json.

Here is screenshot: http://ia.gs/GKuM

@mikeal mikeal merged commit 72b4f00 into mikeal:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 27, 2012
  1. @alexindigo
Commits on May 3, 2012
  1. @alexindigo
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 3 deletions.
  1. +7 −3 index.js
View
10 index.js
@@ -567,9 +567,13 @@ function JSONRequestHandler (req, resp) {
return orig.call(resp, chunk)
}
if (req.method === "PUT" || req.method === "POST") {
- if (req.headers['content-type'] === 'application/json') {
+ if (req.headers['content-type'].split(';')[0] === 'application/json') {
req.on('body', function (body) {
- req.emit('json', JSON.parse(body))
+ try {
+ req.emit('json', JSON.parse(body));
+ } catch (e) {
+ req.emit('error', e);
+ }
})
}
}
@@ -637,7 +641,7 @@ function Route (path, application) {
if (req.method !== 'PUT' && req.method !== 'POST') {
var cc = req.accept.apply(req, keys)
} else {
- var cc = req.headers['content-type']
+ var cc = req.headers['content-type'].split(';')[0];
}
if (!cc) return returnEarly(req, resp, keys, authHandler)
Something went wrong with that request. Please try again.