crawlee icon indicating copy to clipboard operation
crawlee copied to clipboard

Create specialized and contextual errors

Open pocesar opened this issue 5 years ago • 1 comments

Following the post on https://github.com/apifytech/apify-js/pull/642#issuecomment-608586477 I'm created a separated issue for Error improvements.

By creating rich errors, we can help with debugging code with some actual context of when the issue happened. Specialized errors instead of throw new Error(string) could make some things come true:

  • Only log the errors you're interested in, either by using e.name === 'DatasetError' or doing a straight-up e instanceof DatasetError for example.
  • Errors can have information about the current environment from Apify.getEnv(), time that happened.
  • They should have their important meta information serializable to JSON, so they can be pushed to a separate dataset, if needed (I can provide a named dataset that I used for a client if needed).

example implementation: https://github.com/pocesar/apify-facebook-crawler/blob/master/src/error.ts#L46

instead of bikeshedding about errors names, let's focus on what should the error contain. my initial list for everything-Apify:

  • runId = id or null (for local runs)
  • actorId = id or null (for local runs)
  • time = iso datetime string
  • originalError = { message: string, stack: string }
  • context = { [index: string]: any } Anything important related: variables, urls, request/session object, config options, etc

The errors would extend a ApifyBaseError so when an error happens, you can do a instanceof to see if it's an internal error or an external (from dependencies, code / syntax errors, etc)

pocesar avatar Apr 06 '20 01:04 pocesar

Great kick-off. I pretty much agree with everything. Here are a few notes of my own.

The number one thing users hate about our error management is the "internal" logging of SDK (i.e., not manageable by the user). Such as request timed out and will be retried. They would like to be able to turn this logging off, replace it with a warning, or at least remove the stack from it. There's even an attempted PR, but we were not completely happy with it.

We need to design a way to make this customizable via logger. But that needs the ability to select which errors to log. Perhaps via instanceof checks in the log.exception function and then providing the logger instance with a list of exceptions you are / are not interested in logging.

mnmkng avatar Apr 06 '20 08:04 mnmkng