Unify the use of JSON.parse and catch errors.

RESOLVED FIXED

Status

Hello (Loop)
Server
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: alexis, Assigned: natim)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa?])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
It looks like we have 12 instances (in 9 files) to JSON.parse() in this repo, ?q="JSON.parse"&type=Code

We'd probably want to wrap them all in try...catch blocks, or at least the ones not in the /test/ dir.

    /loop/encrypt.js:47
    /loop/fxa.js:27
    /loop/routes/calls.js:252
    /loop/storage/redis.js:141
    /loop/storage/redis.js:198
    /loop/websockets.js:358
    /test/functional_test.js:1178
    /test/middlewares_test.js:58
    /test/storage_test.js:236
    /test/storage_test.js:260
    /test/websockets_test.js:74
    /test/websockets_test.js:90
Whiteboard: [qa?]
(Assignee)

Comment 1

4 years ago
I checked them and all of them are find:

 - For redis we are sure it is JSON 
 - For encrypt it is throwing an error handled later
 - For fxa, I have made a patch for it.
 - For calls it is a JSON.parse(JSON.stringify())
 - For tests we want the error to be thrown.
 - For websocket it is handled on the decode call.
Assignee: nobody → rhubscher
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Created attachment 8479660 [details] [review]
Link to Github PR — #193
Attachment #8479660 - Flags: review?(alexis+bugs)
(Reporter)

Updated

4 years ago
Attachment #8479660 - Flags: review?(alexis+bugs) → review+
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.