Skip to content

Commit

Permalink
notification: Have fromAPNs accept a nullish value.
Browse files Browse the repository at this point in the history
And throw an error if it gets one, since it definitely indicates a
bug. See discussion at
  zulip#4163 (comment).
  • Loading branch information
chrisbobbe committed Jun 17, 2021
1 parent f5ff635 commit 6949325
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
14 changes: 11 additions & 3 deletions src/notification/extract.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,18 @@ class ApnsMsgValidationError extends Error {
// @returns A `Notification` on success, `undefined` on suppressible failure.
// @throws An ApnsMsgValidationError on unexpected failure.
//
export const fromAPNsImpl = (rawData: JSONableDict): Notification | void => {
export const fromAPNsImpl = (rawData: ?JSONableDict): Notification | void => {
/** Helper function: fail. */
const err = (style: string) =>
new ApnsMsgValidationError(`Received ${style} APNs notification`, { data: rawData });
new ApnsMsgValidationError(`Received ${style} APNs notification`, {
// an `undefined` value would make `extras` not JSONable, but we will
// want to know if the value is undefined
data: rawData === undefined ? '__undefined__' : rawData,
});

if (rawData == null) {
throw err('nullish');
}

// APNs messages are JSON dictionaries. The `aps` entry of this dictionary is
// required, with a structure defined by Apple; all other entries are
Expand Down Expand Up @@ -208,7 +216,7 @@ export const fromAPNsImpl = (rawData: JSONableDict): Notification | void => {
*
* @returns A `Notification` on success; `undefined` on failure.
*/
export const fromAPNs = (data: JSONableDict): Notification | void => {
export const fromAPNs = (data: ?JSONableDict): Notification | void => {
try {
return fromAPNsImpl(data);
} catch (err) {
Expand Down
6 changes: 0 additions & 6 deletions src/notification/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,6 @@ const readInitialNotification = async (): Promise<Notification | null> => {
// present, it must be a JSONable dictionary. It's giving us the
// notification data, which was passed over APNs as JSON.
const data: ?JSONableDict = notification.getData();
if (!data) {
return null;
}
return fromAPNs(data) || null;
};

Expand Down Expand Up @@ -265,9 +262,6 @@ export class NotificationListener {
// if present, it must be a JSONable dictionary. It's giving us the
// notification data, which was passed over APNs as JSON.
const data: ?JSONableDict = notification.getData();
if (!data) {
return;
}
const dataFromAPNs: Notification | void = fromAPNs(data);
if (!dataFromAPNs) {
return;
Expand Down

0 comments on commit 6949325

Please sign in to comment.