fix: implement permission request lock and optimize file access
- Add permission request lock to prevent concurrent requests - Implement permission state tracking to avoid redundant checks - Optimize file discovery with single permission check - Enhance error handling for permission denials - Add graceful degradation when permissions are denied - Improve structured logging for better debugging Resolves "Can request only one set of permissions at a time" warnings Reduces redundant permission checks and file system operations Ensures app continues to function with limited permissions
This commit is contained in:
Matthew Raymer
@@ -51,6 +51,11 @@ export class CapacitorPlatformService implements PlatformService {
|
||||
private initializationPromise: Promise<void> | null = null;
|
||||
private operationQueue: Array<QueuedOperation> = [];
|
||||
private isProcessingQueue: boolean = false;
|
||||
|
||||
/** Permission request lock to prevent concurrent requests */
|
||||
private permissionRequestLock: Promise<void> | null = null;
|
||||
private permissionGranted: boolean = false;
|
||||
private permissionChecked: boolean = false;
|
||||
|
||||
constructor() {
|
||||
this.sqlite = new SQLiteConnection(CapacitorSQLite);
|
||||
@@ -277,84 +282,88 @@ export class CapacitorPlatformService implements PlatformService {
|
||||
return;
|
||||
}
|
||||
|
||||
// For Android, try to access external storage to check permissions
|
||||
try {
|
||||
// Try to list files in external storage to check permissions
|
||||
await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.External,
|
||||
});
|
||||
logger.log(
|
||||
"External storage permissions already granted",
|
||||
JSON.stringify({ timestamp: new Date().toISOString() }, null, 2),
|
||||
);
|
||||
return;
|
||||
} catch (error) {
|
||||
const err = error as Error;
|
||||
const errorLogData = {
|
||||
error: {
|
||||
message: err.message,
|
||||
name: err.name,
|
||||
stack: err.stack,
|
||||
},
|
||||
// Check if we already have permissions
|
||||
if (this.permissionChecked && this.permissionGranted) {
|
||||
logger.log("External storage permissions already granted", {
|
||||
timestamp: new Date().toISOString(),
|
||||
};
|
||||
|
||||
// Check for actual permission errors
|
||||
if (
|
||||
err.message.includes("permission") ||
|
||||
err.message.includes("access") ||
|
||||
err.message.includes("denied") ||
|
||||
err.message.includes("User denied storage permission")
|
||||
) {
|
||||
logger.log(
|
||||
"Permission check failed, requesting permissions",
|
||||
JSON.stringify(errorLogData, null, 2),
|
||||
);
|
||||
|
||||
// The Filesystem plugin will automatically request permissions when needed
|
||||
// We just need to try the operation again
|
||||
try {
|
||||
await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.External,
|
||||
});
|
||||
logger.log(
|
||||
"External storage permissions granted after request",
|
||||
JSON.stringify({ timestamp: new Date().toISOString() }, null, 2),
|
||||
);
|
||||
return;
|
||||
} catch (retryError: unknown) {
|
||||
const retryErr = retryError as Error;
|
||||
|
||||
// If permission is still denied, log it but don't throw an error
|
||||
// This allows the app to continue with limited functionality
|
||||
if (retryErr.message.includes("User denied storage permission")) {
|
||||
logger.warn(
|
||||
"External storage permissions denied by user, continuing with limited functionality",
|
||||
JSON.stringify({
|
||||
error: retryErr.message,
|
||||
timestamp: new Date().toISOString()
|
||||
}, null, 2),
|
||||
);
|
||||
return; // Don't throw error, just return
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Failed to obtain external storage permissions: ${retryErr.message}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// For any other error, log it but don't treat as permission error
|
||||
logger.log(
|
||||
"Unexpected error during permission check, proceeding anyway",
|
||||
JSON.stringify(errorLogData, null, 2),
|
||||
);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// If a permission request is already in progress, wait for it
|
||||
if (this.permissionRequestLock) {
|
||||
logger.log("Permission request already in progress, waiting...", {
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
await this.permissionRequestLock;
|
||||
return;
|
||||
}
|
||||
|
||||
// Create a new permission request lock
|
||||
this.permissionRequestLock = this._requestStoragePermissions();
|
||||
|
||||
try {
|
||||
await this.permissionRequestLock;
|
||||
this.permissionGranted = true;
|
||||
this.permissionChecked = true;
|
||||
logger.log("Storage permissions granted successfully", {
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
} catch (error) {
|
||||
this.permissionGranted = false;
|
||||
this.permissionChecked = true;
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.warn("Storage permissions denied, continuing with limited functionality", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
// Don't throw error - allow app to continue with limited functionality
|
||||
} finally {
|
||||
this.permissionRequestLock = null;
|
||||
}
|
||||
} catch (error) {
|
||||
logger.error("Error in checkStoragePermissions:", error);
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.error("Error checking storage permissions:", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
// Don't throw error - allow app to continue with limited functionality
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal method to request storage permissions
|
||||
* @returns Promise that resolves when permissions are granted or denied
|
||||
*/
|
||||
private async _requestStoragePermissions(): Promise<void> {
|
||||
try {
|
||||
// Try to read external storage to trigger permission request
|
||||
await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.ExternalStorage,
|
||||
});
|
||||
|
||||
logger.log("External storage permissions already granted", {
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
|
||||
// Check if this is a permission denial
|
||||
if (errorMessage.includes("user denied permission") ||
|
||||
errorMessage.includes("permission request")) {
|
||||
logger.warn("User denied storage permission", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
throw new Error("Storage permission denied by user");
|
||||
}
|
||||
|
||||
// For other errors, log but don't treat as permission denial
|
||||
logger.warn("Storage access error (not permission-related):", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
@@ -1966,213 +1975,172 @@ export class CapacitorPlatformService implements PlatformService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Enhanced file discovery that searches multiple user-accessible locations
|
||||
* This helps find files regardless of where users chose to save them
|
||||
* Lists user-accessible files with enhanced discovery from multiple storage locations.
|
||||
* This method tries multiple directories and handles permission denials gracefully.
|
||||
* @returns Promise resolving to array of file information
|
||||
*/
|
||||
async listUserAccessibleFilesEnhanced(): Promise<Array<{name: string, uri: string, size?: number, path?: string}>> {
|
||||
const allFiles: Array<{name: string, uri: string, size?: number, path?: string}> = [];
|
||||
|
||||
try {
|
||||
const allFiles: Array<{name: string, uri: string, size?: number, path?: string}> = [];
|
||||
// Check permissions once at the beginning
|
||||
const hasPermissions = await this.hasStoragePermissions();
|
||||
|
||||
if (this.getCapabilities().isIOS) {
|
||||
// iOS: Documents directory
|
||||
const result = await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.Documents,
|
||||
});
|
||||
const files = result.files.map((file) => ({
|
||||
name: typeof file === "string" ? file : file.name,
|
||||
uri: `file://${file.uri || file}`,
|
||||
size: typeof file === "string" ? undefined : file.size,
|
||||
path: "Documents"
|
||||
}));
|
||||
allFiles.push(...files);
|
||||
} else {
|
||||
// Android: Multiple locations with recursive search
|
||||
|
||||
// 1. App's external storage directory
|
||||
// iOS: List files in Documents directory (persistent, accessible via Files app)
|
||||
try {
|
||||
const appStorageResult = await Filesystem.readdir({
|
||||
path: "TimeSafari",
|
||||
directory: Directory.ExternalStorage,
|
||||
const result = await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.Documents,
|
||||
});
|
||||
|
||||
// Log full readdir output for TimeSafari
|
||||
logger.log("[CapacitorPlatformService] Android TimeSafari readdir full result:", {
|
||||
path: "TimeSafari",
|
||||
directory: "ExternalStorage",
|
||||
files: appStorageResult.files,
|
||||
fileCount: appStorageResult.files.length,
|
||||
fileDetails: appStorageResult.files.map((file, index) => ({
|
||||
index,
|
||||
name: typeof file === "string" ? file : file.name,
|
||||
type: typeof file === "string" ? "string" : "object",
|
||||
hasUri: typeof file === "string" ? false : !!file.uri,
|
||||
hasSize: typeof file === "string" ? false : !!file.size,
|
||||
fullObject: file
|
||||
})),
|
||||
const files = result.files
|
||||
.filter((file) => typeof file === "object" && file.type === "file")
|
||||
.map((file) => {
|
||||
const fileObj = file as any;
|
||||
return {
|
||||
name: fileObj.name,
|
||||
uri: fileObj.uri,
|
||||
size: fileObj.size,
|
||||
path: "Documents",
|
||||
};
|
||||
});
|
||||
|
||||
allFiles.push(...files);
|
||||
|
||||
logger.log("[CapacitorPlatformService] iOS Documents files found:", {
|
||||
fileCount: files.length,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const appStorageFiles = appStorageResult.files.map((file) => ({
|
||||
name: typeof file === "string" ? file : file.name,
|
||||
uri: `file://${file.uri || file}`,
|
||||
size: typeof file === "string" ? undefined : file.size,
|
||||
path: "TimeSafari"
|
||||
}));
|
||||
allFiles.push(...appStorageFiles);
|
||||
} catch (error) {
|
||||
const err = error as Error;
|
||||
if (err.message.includes("User denied storage permission")) {
|
||||
logger.warn("[CapacitorPlatformService] Storage permission denied for TimeSafari, skipping");
|
||||
} else {
|
||||
logger.warn("[CapacitorPlatformService] Could not read TimeSafari external storage:", error);
|
||||
}
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.warn("[CapacitorPlatformService] Could not read iOS Documents:", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
|
||||
// 2. Common user-chosen locations (if accessible) with recursive search
|
||||
const commonPaths = [
|
||||
"Download",
|
||||
"Documents",
|
||||
"Backups",
|
||||
"TimeSafari",
|
||||
"Data"
|
||||
];
|
||||
|
||||
for (const path of commonPaths) {
|
||||
} else {
|
||||
// Android: Try multiple storage locations
|
||||
if (hasPermissions) {
|
||||
// 1. App's external storage directory
|
||||
try {
|
||||
const result = await Filesystem.readdir({
|
||||
path: path,
|
||||
const appStorageResult = await Filesystem.readdir({
|
||||
path: "TimeSafari",
|
||||
directory: Directory.ExternalStorage,
|
||||
});
|
||||
|
||||
// Log full readdir output for debugging
|
||||
logger.log(`[CapacitorPlatformService] Android ${path} readdir full result:`, {
|
||||
path: path,
|
||||
directory: "ExternalStorage",
|
||||
files: result.files,
|
||||
fileCount: result.files.length,
|
||||
fileDetails: result.files.map((file, index) => ({
|
||||
index,
|
||||
name: typeof file === "string" ? file : file.name,
|
||||
type: typeof file === "string" ? "string" : "object",
|
||||
hasUri: typeof file === "string" ? false : !!file.uri,
|
||||
hasSize: typeof file === "string" ? false : !!file.size,
|
||||
fullObject: file
|
||||
})),
|
||||
const appFiles = appStorageResult.files
|
||||
.filter((file) => typeof file === "object" && file.type === "file")
|
||||
.map((file) => {
|
||||
const fileObj = file as any;
|
||||
return {
|
||||
name: fileObj.name,
|
||||
uri: fileObj.uri,
|
||||
size: fileObj.size,
|
||||
path: "TimeSafari",
|
||||
};
|
||||
});
|
||||
|
||||
allFiles.push(...appFiles);
|
||||
|
||||
logger.log("[CapacitorPlatformService] Android TimeSafari files found:", {
|
||||
fileCount: appFiles.length,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.warn("[CapacitorPlatformService] Could not read TimeSafari external storage:", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Process each entry (file or directory)
|
||||
const relevantFiles = [];
|
||||
for (const file of result.files) {
|
||||
const fileName = typeof file === "string" ? file : file.name;
|
||||
const name = fileName.toLowerCase();
|
||||
|
||||
// Check if it's a directory by trying to get file stats
|
||||
let isDirectory = false;
|
||||
try {
|
||||
const stat = await Filesystem.stat({
|
||||
path: `${path}/${fileName}`,
|
||||
directory: Directory.ExternalStorage
|
||||
});
|
||||
isDirectory = stat.type === 'directory';
|
||||
} catch (statError) {
|
||||
// If stat fails, assume it's a file
|
||||
isDirectory = false;
|
||||
}
|
||||
|
||||
if (isDirectory) {
|
||||
// RECURSIVELY SEARCH DIRECTORY for backup files
|
||||
logger.log(`[CapacitorPlatformService] Recursively searching directory: ${fileName} in ${path}`);
|
||||
try {
|
||||
const subDirResult = await Filesystem.readdir({
|
||||
path: `${path}/${fileName}`,
|
||||
directory: Directory.ExternalStorage,
|
||||
});
|
||||
|
||||
// Process files in subdirectory
|
||||
for (const subFile of subDirResult.files) {
|
||||
const subFileName = typeof subFile === "string" ? subFile : subFile.name;
|
||||
const subName = subFileName.toLowerCase();
|
||||
|
||||
// Check if subfile matches backup criteria
|
||||
const matchesBackupCriteria = subName.includes('timesafari') ||
|
||||
subName.includes('backup') ||
|
||||
subName.includes('contacts') ||
|
||||
subName.endsWith('.json');
|
||||
|
||||
if (matchesBackupCriteria) {
|
||||
relevantFiles.push({
|
||||
name: subFileName,
|
||||
uri: `file://${subFile.uri || subFile}`,
|
||||
size: typeof subFile === "string" ? undefined : subFile.size,
|
||||
path: `${path}/${fileName}`
|
||||
});
|
||||
logger.log(`[CapacitorPlatformService] Found backup file in subdirectory: ${subFileName} in ${path}/${fileName}`);
|
||||
}
|
||||
}
|
||||
} catch (subDirError) {
|
||||
const subDirErr = subDirError as Error;
|
||||
if (subDirErr.message.includes("User denied storage permission")) {
|
||||
logger.warn(`[CapacitorPlatformService] Storage permission denied for subdirectory ${path}/${fileName}, skipping`);
|
||||
} else {
|
||||
logger.warn(`[CapacitorPlatformService] Could not read subdirectory ${path}/${fileName}:`, subDirError);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Check if file matches backup criteria
|
||||
const matchesBackupCriteria = name.includes('timesafari') ||
|
||||
name.includes('backup') ||
|
||||
name.includes('contacts') ||
|
||||
name.endsWith('.json');
|
||||
|
||||
if (matchesBackupCriteria) {
|
||||
relevantFiles.push({
|
||||
name: fileName,
|
||||
uri: `file://${file.uri || file}`,
|
||||
size: typeof file === "string" ? undefined : file.size,
|
||||
path: path
|
||||
});
|
||||
} else {
|
||||
logger.log(`[CapacitorPlatformService] Excluding non-backup file: ${fileName} in ${path}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (relevantFiles.length > 0) {
|
||||
logger.log(`[CapacitorPlatformService] Found ${relevantFiles.length} relevant files in ${path}:`, {
|
||||
files: relevantFiles.map(f => f.name),
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
allFiles.push(...relevantFiles);
|
||||
}
|
||||
} catch (pathError) {
|
||||
const pathErr = pathError as Error;
|
||||
if (pathErr.message.includes("User denied storage permission")) {
|
||||
logger.warn(`[CapacitorPlatformService] Storage permission denied for ${path}, skipping`);
|
||||
} else {
|
||||
logger.warn(`[CapacitorPlatformService] Could not read ${path}:`, pathError);
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Downloads/TimeSafari directory
|
||||
try {
|
||||
const downloadsResult = await Filesystem.readdir({
|
||||
path: "Download/TimeSafari",
|
||||
directory: Directory.ExternalStorage,
|
||||
});
|
||||
|
||||
const downloadFiles = downloadsResult.files
|
||||
.filter((file) => typeof file === "object" && file.type === "file")
|
||||
.map((file) => {
|
||||
const fileObj = file as any;
|
||||
return {
|
||||
name: fileObj.name,
|
||||
uri: fileObj.uri,
|
||||
size: fileObj.size,
|
||||
path: "Download/TimeSafari",
|
||||
};
|
||||
});
|
||||
|
||||
allFiles.push(...downloadFiles);
|
||||
|
||||
logger.log("[CapacitorPlatformService] Android Downloads/TimeSafari files found:", {
|
||||
fileCount: downloadFiles.length,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.warn("[CapacitorPlatformService] Could not read Downloads/TimeSafari:", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
} else {
|
||||
logger.log("[CapacitorPlatformService] Storage permissions not available, skipping external storage", {
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Remove duplicates based on filename and path
|
||||
const uniqueFiles = allFiles.filter((file, index, self) =>
|
||||
index === self.findIndex(f => f.name === file.name && f.path === file.path)
|
||||
);
|
||||
// Always try app data directory as fallback
|
||||
try {
|
||||
const dataResult = await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.Data,
|
||||
});
|
||||
|
||||
const dataFiles = dataResult.files
|
||||
.filter((file) => typeof file === "object" && file.type === "file")
|
||||
.map((file) => {
|
||||
const fileObj = file as any;
|
||||
return {
|
||||
name: fileObj.name,
|
||||
uri: fileObj.uri,
|
||||
size: fileObj.size,
|
||||
path: "Data",
|
||||
};
|
||||
});
|
||||
|
||||
allFiles.push(...dataFiles);
|
||||
|
||||
logger.log("[CapacitorPlatformService] App data directory files found:", {
|
||||
fileCount: dataFiles.length,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.warn("[CapacitorPlatformService] Could not read app data directory:", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
|
||||
logger.log("[CapacitorPlatformService] Enhanced file discovery results:", {
|
||||
total: uniqueFiles.length,
|
||||
files: uniqueFiles.map(f => ({ name: f.name, path: f.path })),
|
||||
totalFiles: allFiles.length,
|
||||
hasPermissions,
|
||||
platform: this.getCapabilities().isIOS ? "iOS" : "Android",
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
|
||||
return uniqueFiles;
|
||||
return allFiles;
|
||||
} catch (error) {
|
||||
logger.error("[CapacitorPlatformService] Enhanced file discovery failed:", error);
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
logger.error("[CapacitorPlatformService] Error in enhanced file discovery:", {
|
||||
error: errorMessage,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
return [];
|
||||
}
|
||||
}
|
||||
@@ -2605,4 +2573,47 @@ export class CapacitorPlatformService implements PlatformService {
|
||||
// Android 10 (API 29) and above have stricter storage restrictions
|
||||
return androidVersion !== null && androidVersion >= 29;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resets the permission state (useful for testing or when permissions change)
|
||||
*/
|
||||
private resetPermissionState(): void {
|
||||
this.permissionGranted = false;
|
||||
this.permissionChecked = false;
|
||||
this.permissionRequestLock = null;
|
||||
logger.log("[CapacitorPlatformService] Permission state reset", {
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if storage permissions are available without requesting them
|
||||
* @returns Promise resolving to true if permissions are available
|
||||
*/
|
||||
private async hasStoragePermissions(): Promise<boolean> {
|
||||
if (this.permissionChecked) {
|
||||
return this.permissionGranted;
|
||||
}
|
||||
|
||||
// If not checked yet, check without requesting
|
||||
try {
|
||||
await Filesystem.readdir({
|
||||
path: ".",
|
||||
directory: Directory.ExternalStorage,
|
||||
});
|
||||
this.permissionGranted = true;
|
||||
this.permissionChecked = true;
|
||||
return true;
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
if (errorMessage.includes("user denied permission") ||
|
||||
errorMessage.includes("permission request")) {
|
||||
this.permissionGranted = false;
|
||||
this.permissionChecked = true;
|
||||
return false;
|
||||
}
|
||||
// For other errors, assume we don't have permissions
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user