Skip to content

Commit 4684368

Browse files
author
maro114510
committed
fix: Address code review suggestions
- Add CLIENT_ERROR to ErrorCode for 4xx HTTP errors - Improve AUTH_FAILED message clarity - Use FeedlyError in getUserId() for consistency - Clarify validateSettings count validation with Number.isFinite - Prioritize WRONG_PAGE error in handleOpen for better UX - Remove unused errorCode field from error response - Add defense-in-depth comment to validateSender - Add defensive catch comment in message listener - Explicitly send reload setting from popup.js
1 parent aed74e1 commit 4684368

2 files changed

Lines changed: 27 additions & 10 deletions

File tree

content.js

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,18 @@ const ErrorCode = {
1717
RATE_LIMITED: 'RATE_LIMITED',
1818
NETWORK_ERROR: 'NETWORK_ERROR',
1919
SERVER_ERROR: 'SERVER_ERROR',
20+
CLIENT_ERROR: 'CLIENT_ERROR',
2021
WRONG_PAGE: 'WRONG_PAGE',
2122
UNKNOWN: 'UNKNOWN'
2223
};
2324

2425
const UserMessages = {
2526
NO_TOKEN: "Please sign in to Feedly first.",
26-
AUTH_FAILED: "Session expired. Please refresh the page.",
27+
AUTH_FAILED: "Authentication failed. Please sign in to Feedly again.",
2728
RATE_LIMITED: "Too many requests. Please wait a moment.",
2829
NETWORK_ERROR: "Network error. Please check your connection.",
2930
SERVER_ERROR: "Feedly service is temporarily unavailable.",
31+
CLIENT_ERROR: "Invalid request. Please try again.",
3032
WRONG_PAGE: "Please open a Feedly Read Later page.",
3133
UNKNOWN: "Something went wrong. Please try again."
3234
};
@@ -52,6 +54,7 @@ function classifyHttpError(status) {
5254
if (status === 401 || status === 403) return ErrorCode.AUTH_FAILED;
5355
if (status === 429) return ErrorCode.RATE_LIMITED;
5456
if (status >= 500) return ErrorCode.SERVER_ERROR;
57+
if (status >= 400) return ErrorCode.CLIENT_ERROR;
5558
return ErrorCode.UNKNOWN;
5659
}
5760

@@ -271,7 +274,7 @@ async function feedlyApiRequest(endpoint, options = {}) {
271274
async function getUserId() {
272275
const profile = await feedlyApiRequest("/v3/profile");
273276
if (!profile.id) {
274-
throw new Error("User ID not found in profile response");
277+
throw new FeedlyError(ErrorCode.SERVER_ERROR, "User ID not found in profile response");
275278
}
276279
return profile.id;
277280
}
@@ -780,15 +783,20 @@ async function handleOpen(settings) {
780783
console.error("[Feedly Opener] DOM failed:", domError.message);
781784
}
782785

783-
const userMessage = apiError instanceof FeedlyError
784-
? apiError.getUserMessage()
785-
: UserMessages.UNKNOWN;
786+
// Prioritize DOM WRONG_PAGE error as it's more actionable for users
787+
let userMessage;
788+
if (domError instanceof FeedlyError && domError.code === ErrorCode.WRONG_PAGE) {
789+
userMessage = domError.getUserMessage();
790+
} else if (apiError instanceof FeedlyError) {
791+
userMessage = apiError.getUserMessage();
792+
} else {
793+
userMessage = UserMessages.UNKNOWN;
794+
}
786795

787796
return {
788797
ok: false,
789798
error: userMessage,
790-
method: "failed",
791-
errorCode: apiError instanceof FeedlyError ? apiError.code : ErrorCode.UNKNOWN
799+
method: "failed"
792800
};
793801
}
794802
}
@@ -846,6 +854,9 @@ function isRecentlyReadLater() {
846854

847855
/**
848856
* Validate message sender is from our own extension.
857+
* NOTE: This is defense-in-depth. Messages via runtime.onMessage
858+
* should only come from our extension, but we validate explicitly
859+
* to ensure messages are from the expected source.
849860
* @param {Object} sender - Message sender object
850861
* @returns {boolean} True if sender is valid
851862
*/
@@ -860,9 +871,12 @@ function validateSender(sender) {
860871
*/
861872
function validateSettings(raw) {
862873
const validModes = ['all', 'count'];
874+
const rawCount = Number(raw?.count);
875+
const parsedCount = Number.isFinite(rawCount) ? Math.floor(rawCount) : 10;
876+
const safeCount = parsedCount > 0 ? parsedCount : 10;
863877
return {
864878
mode: validModes.includes(raw?.mode) ? raw.mode : 'all',
865-
count: Math.max(1, Math.min(999, Math.floor(Number(raw?.count)) || 10)),
879+
count: Math.max(1, Math.min(999, safeCount)),
866880
reload: typeof raw?.reload === 'boolean' ? raw.reload : true
867881
};
868882
}
@@ -889,6 +903,7 @@ if (!window.__feedlyReadLaterOpenerListenerAdded) {
889903

890904
handleOpen(settings)
891905
.then(sendResponse)
906+
// Defensive catch for unexpected errors (normal flow returns result object)
892907
.catch((e) => {
893908
console.error("[Feedly Opener]", e);
894909
const userMessage = e instanceof FeedlyError ? e.getUserMessage() : UserMessages.UNKNOWN;

popup.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ const usesPromises = typeof browser !== "undefined";
44
const SETTINGS_KEY = "feedlyOpenerSettings";
55
const DEFAULT_SETTINGS = {
66
mode: "all",
7-
count: 10
7+
count: 10,
8+
reload: true
89
};
910

1011
const storageArea =
@@ -190,7 +191,8 @@ function readSettingsFromForm() {
190191

191192
return {
192193
mode,
193-
count
194+
count,
195+
reload: DEFAULT_SETTINGS.reload
194196
};
195197
}
196198

0 commit comments

Comments
 (0)