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

refactor: remove unneeded ErrorKinds #3936

Merged
merged 28 commits into from
Feb 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8dc3bde
use Error and URIError
bartlomieju Feb 9, 2020
3504b46
Merge branch 'master' into error_refactor
bartlomieju Feb 19, 2020
d4c226e
remove ErrorKinds: TooLarge, InvalidSeekMode
bartlomieju Feb 19, 2020
61300e7
fix integration test output
bartlomieju Feb 19, 2020
d761e61
lint
bartlomieju Feb 19, 2020
9affde9
remove ErrorKind::Diagnostic
bartlomieju Feb 19, 2020
ba0d249
fix test output
bartlomieju Feb 19, 2020
19e8038
remove ErrorKind::JSError
bartlomieju Feb 19, 2020
59bc4a3
Merge branch 'master' into error_refactor
bartlomieju Feb 19, 2020
a4e6a25
fix after merge
bartlomieju Feb 19, 2020
d15f993
rename ErrorKind::UrlError -> URIError
bartlomieju Feb 19, 2020
3b6da48
update module docstring
bartlomieju Feb 19, 2020
5710a04
move ErrorKind to cli/deno_error.rs
bartlomieju Feb 19, 2020
ae65f8c
update todo
bartlomieju Feb 19, 2020
7e0a36c
fix ErrorKind
bartlomieju Feb 19, 2020
d483bdf
remove ErrorKind::WouldBlock
bartlomieju Feb 19, 2020
06f170c
remove debug statement
bartlomieju Feb 19, 2020
54674be
remove ErrorKind::UnixError
bartlomieju Feb 20, 2020
23fecc5
remove UnixError, InvalidInput
bartlomieju Feb 20, 2020
45863e8
fmt
bartlomieju Feb 20, 2020
2b4ee74
lint
bartlomieju Feb 20, 2020
20dfa2e
remove Deno.ErrorKind and Deno.Error
bartlomieju Feb 20, 2020
ca1b82a
use Deno.Err
bartlomieju Feb 20, 2020
4871a0c
lint
bartlomieju Feb 20, 2020
82e4a95
fmt
bartlomieju Feb 20, 2020
1a7f22a
fix tests
bartlomieju Feb 20, 2020
d9fc8e3
order ErrorKind
bartlomieju Feb 20, 2020
f756512
fix std tests
bartlomieju Feb 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 73 additions & 135 deletions cli/deno_error.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,34 @@
// 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
//!
//! There are many types of errors in Deno:
//! - ErrBox: a generic boxed object. This is the super type of all
//! errors handled in Rust.
//! - JSError: exceptions thrown from V8 into Rust. Usually a user exception.
//! These are basically a big JSON structure which holds information about
//! line numbers. We use this to pretty-print stack traces. These are
//! never passed back into the runtime.
//! - DenoError: these are errors that happen during ops, which are passed
//! back into the runtime, where an exception object is created and thrown.
//! DenoErrors have an integer code associated with them - access this via the kind() method.
//! - Diagnostic: these are errors that originate in TypeScript's compiler.
//! They're similar to JSError, in that they have line numbers.
//! But Diagnostics are compile-time type errors, whereas JSErrors are runtime exceptions.
//!
//! TODO:
//! - rename DenoError to OpError?
//! - rename JSError to RuntimeException. merge V8Exception?
//! - rename ErrorKind::Other. This corresponds to a generic exception thrown as the
//! global `Error` in JS:
//! https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error

use crate::import_map::ImportMapError;
pub use crate::msg::ErrorKind;
use deno_core::AnyError;
use deno_core::ErrBox;
use deno_core::ModuleResolutionError;
Expand All @@ -16,6 +42,34 @@ use std::fmt;
use std::io;
use url;

// Warning! The values in this enum are duplicated in js/errors.ts
// Update carefully!
#[allow(non_camel_case_types)]
#[repr(i8)]
#[derive(Clone, Copy, PartialEq, Debug)]
pub enum ErrorKind {
NotFound = 1,
PermissionDenied = 2,
ConnectionRefused = 3,
ConnectionReset = 4,
ConnectionAborted = 5,
NotConnected = 6,
AddrInUse = 7,
AddrNotAvailable = 8,
BrokenPipe = 9,
AlreadyExists = 10,
InvalidData = 13,
TimedOut = 14,
Interrupted = 15,
WriteZero = 16,
UnexpectedEof = 17,
BadResource = 18,
Http = 19,
URIError = 20,
TypeError = 21,
Other = 22,
}

#[derive(Debug)]
pub struct DenoError {
kind: ErrorKind,
Expand Down Expand Up @@ -76,11 +130,11 @@ pub fn permission_denied_msg(msg: String) -> ErrBox {
}

pub fn no_buffer_specified() -> ErrBox {
StaticError(ErrorKind::InvalidInput, "no buffer specified").into()
StaticError(ErrorKind::TypeError, "no buffer specified").into()
}

pub fn invalid_address_syntax() -> ErrBox {
StaticError(ErrorKind::InvalidInput, "invalid address syntax").into()
StaticError(ErrorKind::TypeError, "invalid address syntax").into()
}

pub fn other_error(msg: String) -> ErrBox {
Expand All @@ -103,18 +157,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
Expand All @@ -123,12 +165,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::URIError
}
}

Expand Down Expand Up @@ -156,21 +193,21 @@ impl GetErrorKind for io::Error {
AddrNotAvailable => ErrorKind::AddrNotAvailable,
BrokenPipe => ErrorKind::BrokenPipe,
AlreadyExists => ErrorKind::AlreadyExists,
WouldBlock => ErrorKind::WouldBlock,
InvalidInput => ErrorKind::InvalidInput,
InvalidInput => ErrorKind::TypeError,
InvalidData => ErrorKind::InvalidData,
TimedOut => ErrorKind::TimedOut,
Interrupted => ErrorKind::Interrupted,
WriteZero => ErrorKind::WriteZero,
UnexpectedEof => ErrorKind::UnexpectedEof,
WouldBlock => unreachable!(),
_ => ErrorKind::Other,
}
}
}

impl GetErrorKind for url::ParseError {
fn kind(&self) -> ErrorKind {
ErrorKind::UrlParse
ErrorKind::URIError
}
}

Expand Down Expand Up @@ -211,8 +248,8 @@ impl GetErrorKind for serde_json::error::Error {
fn kind(&self) -> ErrorKind {
use serde_json::error::*;
match self.classify() {
Category::Io => ErrorKind::InvalidInput,
Category::Syntax => ErrorKind::InvalidInput,
Category::Io => ErrorKind::TypeError,
Category::Syntax => ErrorKind::TypeError,
Category::Data => ErrorKind::InvalidData,
Category::Eof => ErrorKind::UnexpectedEof,
}
Expand All @@ -230,10 +267,13 @@ mod unix {
fn kind(&self) -> ErrorKind {
match self {
Sys(EPERM) => ErrorKind::PermissionDenied,
Sys(EINVAL) => ErrorKind::InvalidInput,
Sys(EINVAL) => ErrorKind::TypeError,
Sys(ENOENT) => ErrorKind::NotFound,
Sys(_) => ErrorKind::UnixError,
_ => ErrorKind::Other,
Sys(UnknownErrno) => unreachable!(),
Sys(_) => unreachable!(),
Error::InvalidPath => ErrorKind::TypeError,
Error::InvalidUtf8 => ErrorKind::InvalidData,
Error::UnsupportedOperation => unreachable!(),
}
}
}
Expand Down Expand Up @@ -268,11 +308,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))
Expand All @@ -294,93 +332,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)
Expand Down Expand Up @@ -414,26 +366,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::URIError);
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());
Expand Down Expand Up @@ -466,7 +404,7 @@ mod tests {
#[test]
fn test_no_buffer_specified() {
let err = no_buffer_specified();
assert_eq!(err.kind(), ErrorKind::InvalidInput);
assert_eq!(err.kind(), ErrorKind::TypeError);
assert_eq!(err.to_string(), "no buffer specified");
}
}
2 changes: 1 addition & 1 deletion cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::URIError,
"File URL contains invalid path".to_owned(),
))
})?;
Expand Down
12 changes: 4 additions & 8 deletions cli/js/buffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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 {
Expand All @@ -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> {
Expand Down
6 changes: 3 additions & 3 deletions cli/js/buffer_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down
Loading