Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Better error handling for 'native' plugins. #2443

Open
2 of 4 tasks
TaaBooThree opened this issue Feb 17, 2020 · 8 comments
Open
2 of 4 tasks

feat: Better error handling for 'native' plugins. #2443

TaaBooThree opened this issue Feb 17, 2020 · 8 comments
Labels
enhancement needs discussion decisions must be made before working on it

Comments

@TaaBooThree
Copy link

Feature Request

Describe the Feature Request

Hey guys, we are running into issues when handling plugins on native devices.

For example the Geolocation.getCurrentLocation() function returns a different error object on web than it does on native devices.

The error object on this particular function on native devices currently only holds a message property. Without any documentation, it is hard to see what kind of errors it may throw and to write code around handling these errors.

The error object on web is the GeolocationPositionError. It's a lot easier to write different methods around the types of errors it can throw.

The same goes for other plugins like the Camera plugin.

Platform Support Requested

  • Android
  • iOS
  • Electron
  • Web

Describe Preferred Solution

The preferred solution would be a single error object that we can use to handle the error in the same way across platforms. It would also help to have documentation of the errors that certain functions throw. I currently have to search for the error types in the @capacitor/core package.

@jcesarmobile jcesarmobile added this to the 2.0.0 milestone Feb 21, 2020
@jcesarmobile jcesarmobile added the needs discussion decisions must be made before working on it label Feb 21, 2020
@jcesarmobile
Copy link
Member

javascript errors can have a code number and a few other properties, so this could be done

Adding the needs discussion label

@jcesarmobile jcesarmobile removed this from the 2.0.0 milestone Mar 6, 2020
@diachedelic
Copy link
Contributor

It is difficult to categorise Filesystem errors across different platforms, for example:

try {
  await Filesystem.stat({ directory, path })
} catch (err) {
  const androidFileNotFound = err.message === 'File does not exist'
  const iosFileNotFound = err.message.includes('was not found')

  if (androidFileNotFound || iosFileNotFound) {
    return null
  } else {
    // unknown error
    throw err
  }
}

Perhaps well documented err.code properties could be implemented?

@Gorshtak

This comment was marked as abuse.

@TaaBooThree
Copy link
Author

It is difficult to categorise Filesystem errors across different platforms, for example:

try {
  await Filesystem.stat({ directory, path })
} catch (err) {
  const androidFileNotFound = err.message === 'File does not exist'
  const iosFileNotFound = err.message.includes('was not found')

  if (androidFileNotFound || iosFileNotFound) {
    return null
  } else {
    // unknown error
    throw err
  }
}

Perhaps well documented err.code properties could be implemented?

I'd like to see a general error for a file not found scenario as well as the actual platform specific error. Some people might want to handle platform specific errors after all.

One thing that would really help is just documentation around the errors object it throws and the possible messages you can get. I have no idea what kind of error scenarios I can expect and how to handle them properly without going through the source code.

@thoro23
Copy link

thoro23 commented Dec 10, 2021

Reviving this feture request because this is absolutely needed. For example I want to delete a file. If the file does not exist its ok because I wanted to delete it anyway. But I want an display an error message if I do not have the permissions to delete it.

try {
  await Filesystem.deleteFile({ directory, path });
catch (err) {
  // file not found
  if (err.code === 1) {
    return;
  }

  alert(err.message);
}

@robingenz
Copy link
Contributor

It is difficult to categorise Filesystem errors across different platforms, for example:

try {
  await Filesystem.stat({ directory, path })
} catch (err) {
  const androidFileNotFound = err.message === 'File does not exist'
  const iosFileNotFound = err.message.includes('was not found')

  if (androidFileNotFound || iosFileNotFound) {
    return null
  } else {
    // unknown error
    throw err
  }
}

Perhaps well documented err.code properties could be implemented?

Even this solution unfortunately only works if the device is set to English (please correct me if I am wrong).
A customer has set his device to German and the plugin throws the following error if a file does not exist when deleting: Die Datei "image.png" konnte nicht geöffnet werden, da die Datei nicht existiert.
This makes it almost impossible to deal with specific errors at the moment.

@diachedelic
Copy link
Contributor

I just assume that every error is a File Not Found. But this is a little bit dangerous because it could result in an existing file being inadvertently overwritten.

@aaronclawrence
Copy link

FWIW I think that all plugins produce the same error object, which is basically

export interface CapacitorPluginError {
    message: string;
    code: string; // usually not specified
    data: object; // additional data, usually not specified
}

So it's possible for plugins to supply a code that would presumably be easier to use, but most don't do so, they only supply the message. In most plugins it's a hard-coded English string,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs discussion decisions must be made before working on it
Projects
None yet
Development

No branches or pull requests

7 participants