refactor: remove unneeded ErrorKinds

cli/deno_error.rs

@@ -1,6 +1,18 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-use crate::diagnostics::Diagnostic;
-use crate::fmt_errors::JSError;
+
+/// This module implements error serialization; it
+/// allows to serialize Rust errors to be sent to JS runtime.
+///
+/// Currently it is deeply intertwined with `ErrBox` which is
+/// not optimal since not every ErrBox can be "JS runtime error";
+/// eg. there's no way to throw JSError/Diagnostic from within JS runtime
+///
+/// TODO:
+///  - rename ErrorKind::Other to WindowError/GenericError
+///  - remove ErrorKind::WouldBlock
+///  - possibly rename "GetErrorKind" to "RuntimeError" - it will allow
+///    better semantic separation of errors which can be sent to JS runtime
+///    eg. each RuntimeError is ErrBox, but not every ErrBox is RuntimeError
 use crate::import_map::ImportMapError;
 pub use crate::msg::ErrorKind;
 use deno_core::AnyError;
@@ -103,18 +115,6 @@ impl GetErrorKind for StaticError {
   }
 }
 
-impl GetErrorKind for JSError {
-  fn kind(&self) -> ErrorKind {
-    ErrorKind::JSError
-  }
-}
-
-impl GetErrorKind for Diagnostic {
-  fn kind(&self) -> ErrorKind {
-    ErrorKind::Diagnostic
-  }
-}
-
 impl GetErrorKind for ImportMapError {
   fn kind(&self) -> ErrorKind {
     ErrorKind::Other
@@ -123,12 +123,7 @@ impl GetErrorKind for ImportMapError {
 
 impl GetErrorKind for ModuleResolutionError {
   fn kind(&self) -> ErrorKind {
-    use ModuleResolutionError::*;
-    match self {
-      InvalidUrl(ref err) | InvalidBaseUrl(ref err) => err.kind(),
-      InvalidPath(_) => ErrorKind::InvalidPath,
-      ImportPrefixMissing(_, _) => ErrorKind::ImportPrefixMissing,
-    }
+    ErrorKind::UrlError
   }
 }
 
@@ -170,7 +165,7 @@ impl GetErrorKind for io::Error {
 
 impl GetErrorKind for url::ParseError {
   fn kind(&self) -> ErrorKind {
-    ErrorKind::UrlParse
+    ErrorKind::UrlError
   }
 }
 
@@ -252,6 +247,8 @@ impl GetErrorKind for DlopenError {
   }
 }
 
+// NOTE(bartlomieju): seems this is necessary - can't use ErrBox here
+// TODO(bartlomieju): ultimately this should be rewritten to `RuntimeError`?
 impl GetErrorKind for dyn AnyError {
   fn kind(&self) -> ErrorKind {
     use self::GetErrorKind as Get;
@@ -268,11 +265,9 @@ impl GetErrorKind for dyn AnyError {
 
     None
       .or_else(|| self.downcast_ref::<DenoError>().map(Get::kind))
-      .or_else(|| self.downcast_ref::<Diagnostic>().map(Get::kind))
       .or_else(|| self.downcast_ref::<reqwest::Error>().map(Get::kind))
       .or_else(|| self.downcast_ref::<ImportMapError>().map(Get::kind))
       .or_else(|| self.downcast_ref::<io::Error>().map(Get::kind))
-      .or_else(|| self.downcast_ref::<JSError>().map(Get::kind))
       .or_else(|| self.downcast_ref::<ModuleResolutionError>().map(Get::kind))
       .or_else(|| self.downcast_ref::<StaticError>().map(Get::kind))
       .or_else(|| self.downcast_ref::<url::ParseError>().map(Get::kind))
@@ -294,93 +289,7 @@ impl GetErrorKind for dyn AnyError {
 #[cfg(test)]
 mod tests {
   use super::*;
-  use crate::colors::strip_ansi_codes;
-  use crate::diagnostics::Diagnostic;
-  use crate::diagnostics::DiagnosticCategory;
-  use crate::diagnostics::DiagnosticItem;
   use deno_core::ErrBox;
-  use deno_core::StackFrame;
-  use deno_core::V8Exception;
-
-  fn js_error() -> JSError {
-    JSError::new(V8Exception {
-      message: "Error: foo bar".to_string(),
-      source_line: None,
-      script_resource_name: None,
-      line_number: None,
-      start_position: None,
-      end_position: None,
-      error_level: None,
-      start_column: None,
-      end_column: None,
-      frames: vec![
-        StackFrame {
-          line: 4,
-          column: 16,
-          script_name: "foo_bar.ts".to_string(),
-          function_name: "foo".to_string(),
-          is_eval: false,
-          is_constructor: false,
-          is_wasm: false,
-        },
-        StackFrame {
-          line: 5,
-          column: 20,
-          script_name: "bar_baz.ts".to_string(),
-          function_name: "qat".to_string(),
-          is_eval: false,
-          is_constructor: false,
-          is_wasm: false,
-        },
-        StackFrame {
-          line: 1,
-          column: 1,
-          script_name: "deno_main.js".to_string(),
-          function_name: "".to_string(),
-          is_eval: false,
-          is_constructor: false,
-          is_wasm: false,
-        },
-      ],
-    })
-  }
-
-  fn diagnostic() -> Diagnostic {
-    Diagnostic {
-      items: vec![
-        DiagnosticItem {
-          message: "Example 1".to_string(),
-          message_chain: None,
-          code: 2322,
-          category: DiagnosticCategory::Error,
-          start_position: Some(267),
-          end_position: Some(273),
-          source_line: Some("  values: o => [".to_string()),
-          line_number: Some(18),
-          script_resource_name: Some(
-            "deno/tests/complex_diagnostics.ts".to_string(),
-          ),
-          start_column: Some(2),
-          end_column: Some(8),
-          related_information: None,
-        },
-        DiagnosticItem {
-          message: "Example 2".to_string(),
-          message_chain: None,
-          code: 2000,
-          category: DiagnosticCategory::Error,
-          start_position: Some(2),
-          end_position: Some(2),
-          source_line: Some("  values: undefined,".to_string()),
-          line_number: Some(128),
-          script_resource_name: Some("/foo/bar.ts".to_string()),
-          start_column: Some(2),
-          end_column: Some(8),
-          related_information: None,
-        },
-      ],
-    }
-  }
 
   fn io_error() -> io::Error {
     io::Error::from(io::ErrorKind::NotFound)
@@ -414,26 +323,12 @@ mod tests {
   #[test]
   fn test_url_error() {
     let err = ErrBox::from(url_error());
-    assert_eq!(err.kind(), ErrorKind::UrlParse);
+    assert_eq!(err.kind(), ErrorKind::UrlError);
     assert_eq!(err.to_string(), "empty host");
   }
 
   // TODO find a way to easily test tokio errors and unix errors
 
-  #[test]
-  fn test_diagnostic() {
-    let err = ErrBox::from(diagnostic());
-    assert_eq!(err.kind(), ErrorKind::Diagnostic);
-    assert_eq!(strip_ansi_codes(&err.to_string()), "error TS2322: Example 1\n\n► deno/tests/complex_diagnostics.ts:19:3\n\n19   values: o => [\n     ~~~~~~\n\nerror TS2000: Example 2\n\n► /foo/bar.ts:129:3\n\n129   values: undefined,\n      ~~~~~~\n\n\nFound 2 errors.\n");
-  }
-
-  #[test]
-  fn test_js_error() {
-    let err = ErrBox::from(js_error());
-    assert_eq!(err.kind(), ErrorKind::JSError);
-    assert_eq!(strip_ansi_codes(&err.to_string()), "error: Error: foo bar\n    at foo (foo_bar.ts:5:17)\n    at qat (bar_baz.ts:6:21)\n    at deno_main.js:2:2");
-  }
-
   #[test]
   fn test_import_map_error() {
     let err = ErrBox::from(import_map_error());

cli/file_fetcher.rs

@@ -267,7 +267,7 @@ impl SourceFileFetcher {
   fn fetch_local_file(&self, module_url: &Url) -> Result<SourceFile, ErrBox> {
     let filepath = module_url.to_file_path().map_err(|()| {
       ErrBox::from(DenoError::new(
-        ErrorKind::InvalidPath,
+        ErrorKind::UrlError,
         "File URL contains invalid path".to_owned(),
       ))
     })?;

cli/js/buffer.ts

@@ -7,7 +7,6 @@
 import { Reader, Writer, EOF, SyncReader, SyncWriter } from "./io.ts";
 import { assert } from "./util.ts";
 import { TextDecoder } from "./text_encoding.ts";
-import { DenoError, ErrorKind } from "./errors.ts";
 
 // MIN_READ is the minimum ArrayBuffer size passed to a read call by
 // buffer.ReadFrom. As long as the Buffer has at least MIN_READ bytes beyond
@@ -162,7 +161,7 @@ export class Buffer implements Reader, SyncReader, Writer, SyncWriter {
 
   /** _grow() grows the buffer to guarantee space for n more bytes.
    * It returns the index where bytes should be written.
-   * If the buffer can't grow it will throw with ErrTooLarge.
+   * If the buffer can't grow it will throw with Error.
    */
   private _grow(n: number): number {
     const m = this.length;
@@ -183,10 +182,7 @@ export class Buffer implements Reader, SyncReader, Writer, SyncWriter {
       // don't spend all our time copying.
       copyBytes(this.buf, this.buf.subarray(this.off));
     } else if (c > MAX_SIZE - c - n) {
-      throw new DenoError(
-        ErrorKind.TooLarge,
-        "The buffer cannot be grown beyond the maximum size."
-      );
+      throw new Error("The buffer cannot be grown beyond the maximum size.");
     } else {
       // Not enough space anywhere, we need to allocate.
       const buf = new Uint8Array(2 * c + n);
@@ -202,7 +198,7 @@ export class Buffer implements Reader, SyncReader, Writer, SyncWriter {
   /** grow() grows the buffer's capacity, if necessary, to guarantee space for
    * another n bytes. After grow(n), at least n bytes can be written to the
    * buffer without another allocation. If n is negative, grow() will panic. If
-   * the buffer can't grow it will throw ErrTooLarge.
+   * the buffer can't grow it will throw Error.
    * Based on https://golang.org/pkg/bytes/#Buffer.Grow
    */
   grow(n: number): void {
@@ -215,7 +211,7 @@ export class Buffer implements Reader, SyncReader, Writer, SyncWriter {
 
   /** readFrom() reads data from r until EOF and appends it to the buffer,
    * growing the buffer as needed. It returns the number of bytes read. If the
-   * buffer becomes too large, readFrom will panic with ErrTooLarge.
+   * buffer becomes too large, readFrom will panic with Error.
    * Based on https://golang.org/pkg/bytes/#Buffer.ReadFrom
    */
   async readFrom(r: Reader): Promise<number> {

cli/js/buffer_test.ts

@@ -3,7 +3,7 @@
 // This code has been ported almost directly from Go's src/bytes/buffer_test.go
 // Copyright 2009 The Go Authors. All rights reserved. BSD license.
 // https://github.com/golang/go/blob/master/LICENSE
-import { assert, assertEquals, test } from "./test_util.ts";
+import { assertEquals, assert, assertStrContains, test } from "./test_util.ts";
 
 const { Buffer, readAll, readAllSync, writeAll, writeAllSync } = Deno;
 type Buffer = Deno.Buffer;
@@ -159,8 +159,8 @@ test(async function bufferTooLargeByteWrites(): Promise<void> {
     err = e;
   }
 
-  assertEquals(err.kind, Deno.ErrorKind.TooLarge);
-  assertEquals(err.name, "TooLarge");
+  assert(err instanceof Error);
+  assertStrContains(err.message, "grown beyond the maximum size");
 });
 
 test(async function bufferLargeByteReads(): Promise<void> {

cli/js/dispatch_json.ts

@@ -2,7 +2,7 @@
 import * as util from "./util.ts";
 import { TextEncoder, TextDecoder } from "./text_encoding.ts";
 import { core } from "./core.ts";
-import { ErrorKind, DenoError } from "./errors.ts";
+import { ErrorKind, constructError } from "./errors.ts";
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 type Ok = any;
@@ -37,7 +37,7 @@ function encode(args: object): Uint8Array {
 
 function unwrapResponse(res: JsonResponse): Ok {
   if (res.err != null) {
-    throw new DenoError(res.err!.kind, res.err!.message);
+    return constructError(res.err!.kind, res.err!.message);
   }
   util.assert(res.ok != null);
   return res.ok;

cli/js/dispatch_minimal.ts

@@ -2,7 +2,7 @@
 import * as util from "./util.ts";
 import { core } from "./core.ts";
 import { TextDecoder } from "./text_encoding.ts";
-import { ErrorKind, DenoError } from "./errors.ts";
+import { ErrorKind, constructError } from "./errors.ts";
 
 const promiseTableMin = new Map<number, util.Resolvable<RecordMinimal>>();
 // Note it's important that promiseId starts at 1 instead of 0, because sync
@@ -56,7 +56,7 @@ export function recordFromBufMinimal(ui8: Uint8Array): RecordMinimal {
 
 function unwrapResponse(res: RecordMinimal): number {
   if (res.err != null) {
-    throw new DenoError(res.err!.kind, res.err!.message);
+    return constructError(res.err!.kind, res.err!.message);
   }
   return res.result;
 }

cli/js/errors.ts

@@ -1,5 +1,18 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
+export function constructError(kind: ErrorKind, msg: string): never {
+  switch (kind) {
+    case ErrorKind.TypeError:
+      throw new TypeError(msg);
+    case ErrorKind.Other:
+      throw new Error(msg);
+    case ErrorKind.UrlError:
+      throw new URIError(msg);
+    default:
+      throw new DenoError(kind, msg);
+  }
+}
+
 /** A Deno specific error.  The `kind` property is set to a specific error code
  * which can be used to in application logic.
  *
@@ -44,13 +57,9 @@ export enum ErrorKind {
   Other = 17,
   UnexpectedEof = 18,
   BadResource = 19,
-  UrlParse = 20,
   Http = 21,
-  TooLarge = 22,
-  InvalidSeekMode = 23,
   UnixError = 24,
-  InvalidPath = 25,
-  ImportPrefixMissing = 26,
-  Diagnostic = 27,
-  JSError = 28
+
+  TypeError = 101,
+  UrlError = 100
 }

cli/js/fetch_test.ts

@@ -107,8 +107,7 @@ testPerm({ net: true }, async function fetchEmptyInvalid(): Promise<void> {
   } catch (err_) {
     err = err_;
   }
-  assertEquals(err.kind, Deno.ErrorKind.UrlParse);
-  assertEquals(err.name, "UrlParse");
+  assert(err instanceof URIError);
 });
 
 testPerm({ net: true }, async function fetchMultipartFormDataSuccess(): Promise<

cli/js/files_test.ts

@@ -380,8 +380,8 @@ testPerm({ read: true }, async function seekMode(): Promise<void> {
     err = e;
   }
   assert(!!err);
-  assertEquals(err.kind, Deno.ErrorKind.InvalidSeekMode);
-  assertEquals(err.name, "InvalidSeekMode");
+  assert(err instanceof TypeError);
+  assertStrContains(err.message, "Invalid seek mode");
 
   // We should still be able to read the file
   // since it is still open.