refactor: reduce number of ErrorKind variants (#3662)

This commit is contained in:
Bartek Iwańczuk 2020-01-20 16:50:16 +01:00 committed by Ry Dahl
parent e83658138b
commit c90036ab88
13 changed files with 61 additions and 224 deletions

2
Cargo.lock generated
View file

@ -265,8 +265,6 @@ dependencies = [
"futures 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
"fwdansi 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
"http 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hyper 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)",
"hyper-rustls 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)",
"indexmap 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)",

View file

@ -36,8 +36,6 @@ dirs = "2.0.2"
dlopen = "0.1.8"
futures = { version = "0.3.1", features = [ "compat", "io-compat" ] }
http = "0.2.0"
hyper = "0.13.1"
hyper-rustls = "0.19.0"
indexmap = "1.3.0"
lazy_static = "1.4.0"
libc = "0.2.66"

View file

@ -7,8 +7,6 @@ use deno_core::AnyError;
use deno_core::ErrBox;
use deno_core::ModuleResolutionError;
use dlopen::Error as DlopenError;
use http::uri;
use hyper;
use reqwest;
use rustyline::error::ReadlineError;
use std;
@ -77,33 +75,16 @@ pub fn permission_denied_msg(msg: String) -> ErrBox {
DenoError::new(ErrorKind::PermissionDenied, msg).into()
}
pub fn op_not_implemented() -> ErrBox {
StaticError(ErrorKind::OpNotAvailable, "op not implemented").into()
}
pub fn no_buffer_specified() -> ErrBox {
StaticError(ErrorKind::InvalidInput, "no buffer specified").into()
}
pub fn no_async_support() -> ErrBox {
StaticError(ErrorKind::NoAsyncSupport, "op doesn't support async calls")
.into()
}
pub fn no_sync_support() -> ErrBox {
StaticError(ErrorKind::NoSyncSupport, "op doesn't support sync calls").into()
}
pub fn invalid_address_syntax() -> ErrBox {
StaticError(ErrorKind::InvalidInput, "invalid address syntax").into()
}
pub fn too_many_redirects() -> ErrBox {
StaticError(ErrorKind::TooManyRedirects, "too many redirects").into()
}
pub fn type_error(msg: String) -> ErrBox {
DenoError::new(ErrorKind::TypeError, msg).into()
pub fn other_error(msg: String) -> ErrBox {
DenoError::new(ErrorKind::Other, msg).into()
}
pub trait GetErrorKind {
@ -136,7 +117,7 @@ impl GetErrorKind for Diagnostic {
impl GetErrorKind for ImportMapError {
fn kind(&self) -> ErrorKind {
ErrorKind::ImportMapError
ErrorKind::Other
}
}
@ -187,44 +168,9 @@ impl GetErrorKind for io::Error {
}
}
impl GetErrorKind for uri::InvalidUri {
fn kind(&self) -> ErrorKind {
// The http::uri::ErrorKind exists and is similar to url::ParseError.
// However it is also private, so we can't get any details out.
ErrorKind::InvalidUri
}
}
impl GetErrorKind for url::ParseError {
fn kind(&self) -> ErrorKind {
use url::ParseError::*;
match self {
EmptyHost => ErrorKind::EmptyHost,
IdnaError => ErrorKind::IdnaError,
InvalidDomainCharacter => ErrorKind::InvalidDomainCharacter,
InvalidIpv4Address => ErrorKind::InvalidIpv4Address,
InvalidIpv6Address => ErrorKind::InvalidIpv6Address,
InvalidPort => ErrorKind::InvalidPort,
Overflow => ErrorKind::Overflow,
RelativeUrlWithCannotBeABaseBase => {
ErrorKind::RelativeUrlWithCannotBeABaseBase
}
RelativeUrlWithoutBase => ErrorKind::RelativeUrlWithoutBase,
SetHostOnCannotBeABaseUrl => ErrorKind::SetHostOnCannotBeABaseUrl,
_ => ErrorKind::Other,
}
}
}
impl GetErrorKind for hyper::Error {
fn kind(&self) -> ErrorKind {
match self {
e if e.is_canceled() => ErrorKind::HttpCanceled,
e if e.is_closed() => ErrorKind::HttpClosed,
e if e.is_parse() => ErrorKind::HttpParse,
e if e.is_user() => ErrorKind::HttpUser,
_ => ErrorKind::HttpOther,
}
ErrorKind::UrlParse
}
}
@ -234,7 +180,6 @@ impl GetErrorKind for reqwest::Error {
match self.source() {
Some(err_ref) => None
.or_else(|| err_ref.downcast_ref::<hyper::Error>().map(Get::kind))
.or_else(|| err_ref.downcast_ref::<url::ParseError>().map(Get::kind))
.or_else(|| err_ref.downcast_ref::<io::Error>().map(Get::kind))
.or_else(|| {
@ -242,8 +187,8 @@ impl GetErrorKind for reqwest::Error {
.downcast_ref::<serde_json::error::Error>()
.map(Get::kind)
})
.unwrap_or_else(|| ErrorKind::HttpOther),
None => ErrorKind::HttpOther,
.unwrap_or_else(|| ErrorKind::Http),
None => ErrorKind::Http,
}
}
}
@ -324,14 +269,12 @@ 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::<hyper::Error>().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::<uri::InvalidUri>().map(Get::kind))
.or_else(|| self.downcast_ref::<url::ParseError>().map(Get::kind))
.or_else(|| self.downcast_ref::<VarError>().map(Get::kind))
.or_else(|| self.downcast_ref::<ReadlineError>().map(Get::kind))
@ -456,8 +399,8 @@ mod tests {
#[test]
fn test_simple_error() {
let err =
ErrBox::from(DenoError::new(ErrorKind::NoError, "foo".to_string()));
assert_eq!(err.kind(), ErrorKind::NoError);
ErrBox::from(DenoError::new(ErrorKind::NotFound, "foo".to_string()));
assert_eq!(err.kind(), ErrorKind::NotFound);
assert_eq!(err.to_string(), "foo");
}
@ -471,7 +414,7 @@ mod tests {
#[test]
fn test_url_error() {
let err = ErrBox::from(url_error());
assert_eq!(err.kind(), ErrorKind::EmptyHost);
assert_eq!(err.kind(), ErrorKind::UrlParse);
assert_eq!(err.to_string(), "empty host");
}
@ -494,7 +437,7 @@ mod tests {
#[test]
fn test_import_map_error() {
let err = ErrBox::from(import_map_error());
assert_eq!(err.kind(), ErrorKind::ImportMapError);
assert_eq!(err.kind(), ErrorKind::Other);
assert_eq!(err.to_string(), "an import map error");
}
@ -520,31 +463,10 @@ mod tests {
assert_eq!(err.to_string(), "run again with the --allow-net flag");
}
#[test]
fn test_op_not_implemented() {
let err = op_not_implemented();
assert_eq!(err.kind(), ErrorKind::OpNotAvailable);
assert_eq!(err.to_string(), "op not implemented");
}
#[test]
fn test_no_buffer_specified() {
let err = no_buffer_specified();
assert_eq!(err.kind(), ErrorKind::InvalidInput);
assert_eq!(err.to_string(), "no buffer specified");
}
#[test]
fn test_no_async_support() {
let err = no_async_support();
assert_eq!(err.kind(), ErrorKind::NoAsyncSupport);
assert_eq!(err.to_string(), "op doesn't support async calls");
}
#[test]
fn test_no_sync_support() {
let err = no_sync_support();
assert_eq!(err.kind(), ErrorKind::NoSyncSupport);
assert_eq!(err.to_string(), "op doesn't support sync calls");
}
}

View file

@ -1,5 +1,4 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
use crate::deno_error::too_many_redirects;
use crate::deno_error::DenoError;
use crate::deno_error::ErrorKind;
use crate::deno_error::GetErrorKind;
@ -107,7 +106,7 @@ impl SourceFileFetcher {
if !SUPPORTED_URL_SCHEMES.contains(&url.scheme()) {
return Err(
DenoError::new(
ErrorKind::UnsupportedFetchScheme,
ErrorKind::Other,
format!("Unsupported scheme \"{}\" for module \"{}\". Supported schemes: {:#?}", url.scheme(), url, SUPPORTED_URL_SCHEMES),
).into()
);
@ -358,7 +357,8 @@ impl SourceFileFetcher {
redirect_limit: i64,
) -> Pin<Box<SourceFileFuture>> {
if redirect_limit < 0 {
return futures::future::err(too_many_redirects()).boxed();
let e = DenoError::new(ErrorKind::Http, "too many redirects".to_string());
return futures::future::err(e.into()).boxed();
}
let is_blacklisted =
@ -1295,7 +1295,7 @@ mod tests {
.map(move |result| {
assert!(result.is_err());
let err = result.err().unwrap();
assert_eq!(err.kind(), ErrorKind::TooManyRedirects);
assert_eq!(err.kind(), ErrorKind::Http);
});
tokio_util::run(fut);
@ -1571,7 +1571,7 @@ mod tests {
SourceFileFetcher::check_if_supported_scheme(&url)
.unwrap_err()
.kind(),
ErrorKind::UnsupportedFetchScheme
ErrorKind::Other
);
}
}

View file

@ -126,7 +126,9 @@ pub fn chown(path: &str, uid: u32, gid: u32) -> Result<(), ErrBox> {
pub fn chown(_path: &str, _uid: u32, _gid: u32) -> Result<(), ErrBox> {
// Noop
// TODO: implement chown for Windows
Err(crate::deno_error::op_not_implemented())
Err(crate::deno_error::other_error(
"Op not implemented".to_string(),
))
}
pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, ErrBox> {

View file

@ -8,9 +8,9 @@
* } catch (e) {
* if (
* e instanceof Deno.DenoError &&
* e.kind === Deno.ErrorKind.Overflow
* e.kind === Deno.ErrorKind.NotFound
* ) {
* console.error("Overflow error!");
* console.error("NotFound error!");
* }
* }
*
@ -25,7 +25,6 @@ export class DenoError<T extends ErrorKind> extends Error {
// Warning! The values in this enum are duplicated in cli/msg.rs
// Update carefully!
export enum ErrorKind {
NoError = 0,
NotFound = 1,
PermissionDenied = 2,
ConnectionRefused = 3,
@ -45,39 +44,13 @@ export enum ErrorKind {
Other = 17,
UnexpectedEof = 18,
BadResource = 19,
CommandFailed = 20,
EmptyHost = 21,
IdnaError = 22,
InvalidPort = 23,
InvalidIpv4Address = 24,
InvalidIpv6Address = 25,
InvalidDomainCharacter = 26,
RelativeUrlWithoutBase = 27,
RelativeUrlWithCannotBeABaseBase = 28,
SetHostOnCannotBeABaseUrl = 29,
Overflow = 30,
HttpUser = 31,
HttpClosed = 32,
HttpCanceled = 33,
HttpParse = 34,
HttpOther = 35,
TooLarge = 36,
InvalidUri = 37,
InvalidSeekMode = 38,
OpNotAvailable = 39,
WorkerInitFailed = 40,
UnixError = 41,
NoAsyncSupport = 42,
NoSyncSupport = 43,
ImportMapError = 44,
InvalidPath = 45,
ImportPrefixMissing = 46,
UnsupportedFetchScheme = 47,
TooManyRedirects = 48,
Diagnostic = 49,
JSError = 50,
TypeError = 51,
/** TODO this is a DomException type, and should be moved out of here when possible */
DataCloneError = 52
UrlParse = 20,
Http = 21,
TooLarge = 22,
InvalidSeekMode = 23,
UnixError = 24,
InvalidPath = 25,
ImportPrefixMissing = 26,
Diagnostic = 27,
JSError = 28
}

View file

@ -15,8 +15,8 @@ testPerm({ net: true }, async function fetchConnectionError(): Promise<void> {
} catch (err_) {
err = err_;
}
assertEquals(err.kind, Deno.ErrorKind.HttpOther);
assertEquals(err.name, "HttpOther");
assertEquals(err.kind, Deno.ErrorKind.Http);
assertEquals(err.name, "Http");
assertStrContains(err.message, "error trying to connect");
});
@ -106,8 +106,8 @@ testPerm({ net: true }, async function fetchEmptyInvalid(): Promise<void> {
} catch (err_) {
err = err_;
}
assertEquals(err.kind, Deno.ErrorKind.RelativeUrlWithoutBase);
assertEquals(err.name, "RelativeUrlWithoutBase");
assertEquals(err.kind, Deno.ErrorKind.UrlParse);
assertEquals(err.name, "UrlParse");
});
testPerm({ net: true }, async function fetchMultipartFormDataSuccess(): Promise<

View file

@ -1114,9 +1114,9 @@ declare namespace Deno {
* } catch (e) {
* if (
* e instanceof Deno.DenoError &&
* e.kind === Deno.ErrorKind.Overflow
* e.kind === Deno.ErrorKind.NotFound
* ) {
* console.error("Overflow error!");
* console.error("NotFound error!");
* }
* }
*
@ -1126,7 +1126,6 @@ declare namespace Deno {
constructor(kind: T, msg: string);
}
export enum ErrorKind {
NoError = 0,
NotFound = 1,
PermissionDenied = 2,
ConnectionRefused = 3,
@ -1146,37 +1145,15 @@ declare namespace Deno {
Other = 17,
UnexpectedEof = 18,
BadResource = 19,
CommandFailed = 20,
EmptyHost = 21,
IdnaError = 22,
InvalidPort = 23,
InvalidIpv4Address = 24,
InvalidIpv6Address = 25,
InvalidDomainCharacter = 26,
RelativeUrlWithoutBase = 27,
RelativeUrlWithCannotBeABaseBase = 28,
SetHostOnCannotBeABaseUrl = 29,
Overflow = 30,
HttpUser = 31,
HttpClosed = 32,
HttpCanceled = 33,
HttpParse = 34,
HttpOther = 35,
TooLarge = 36,
InvalidUri = 37,
InvalidSeekMode = 38,
OpNotAvailable = 39,
WorkerInitFailed = 40,
UnixError = 41,
NoAsyncSupport = 42,
NoSyncSupport = 43,
ImportMapError = 44,
InvalidPath = 45,
ImportPrefixMissing = 46,
UnsupportedFetchScheme = 47,
TooManyRedirects = 48,
Diagnostic = 49,
JSError = 50
UrlParse = 20,
Http = 21,
TooLarge = 22,
InvalidSeekMode = 23,
UnixError = 24,
InvalidPath = 25,
ImportPrefixMissing = 26,
Diagnostic = 27,
JSError = 28
}
/** UNSTABLE: potentially want names to overlap more with browser.

View file

@ -35,15 +35,14 @@ test(async function permissionInvalidName(): Promise<void> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await Deno.permissions.query({ name: "foo" as any });
} catch (e) {
assert(e.name === "TypeError");
assert(e.name === "Other");
}
});
test(async function permissionNetInvalidUrl(): Promise<void> {
try {
// Invalid url causes TypeError.
await Deno.permissions.query({ name: "net", url: ":" });
} catch (e) {
assert(e.name === "TypeError");
assert(e.name === "UrlParse");
}
});

View file

@ -12,7 +12,6 @@
// TODO don't disable this warning
import { AbortSignal, QueuingStrategySizeCallback } from "../dom_types.ts";
import { DenoError, ErrorKind } from "../errors.ts";
// common stream fields
@ -208,10 +207,7 @@ export function cloneValue(value: any): any {
default:
// TODO this should be a DOMException,
// https://github.com/stardazed/sd-streams/blob/master/packages/streams/src/shared-internals.ts#L171
throw new DenoError(
ErrorKind.DataCloneError,
"Uncloneable value in stream"
);
throw new Error("Uncloneable value in stream");
}
}

View file

@ -6,7 +6,6 @@
#[repr(i8)]
#[derive(Clone, Copy, PartialEq, Debug)]
pub enum ErrorKind {
NoError = 0,
NotFound = 1,
PermissionDenied = 2,
ConnectionRefused = 3,
@ -26,41 +25,15 @@ pub enum ErrorKind {
Other = 17,
UnexpectedEof = 18,
BadResource = 19,
CommandFailed = 20,
EmptyHost = 21,
IdnaError = 22,
InvalidPort = 23,
InvalidIpv4Address = 24,
InvalidIpv6Address = 25,
InvalidDomainCharacter = 26,
RelativeUrlWithoutBase = 27,
RelativeUrlWithCannotBeABaseBase = 28,
SetHostOnCannotBeABaseUrl = 29,
Overflow = 30,
HttpUser = 31,
HttpClosed = 32,
HttpCanceled = 33,
HttpParse = 34,
HttpOther = 35,
TooLarge = 36,
InvalidUri = 37,
InvalidSeekMode = 38,
OpNotAvailable = 39,
WorkerInitFailed = 40,
UnixError = 41,
NoAsyncSupport = 42,
NoSyncSupport = 43,
ImportMapError = 44,
InvalidPath = 45,
ImportPrefixMissing = 46,
UnsupportedFetchScheme = 47,
TooManyRedirects = 48,
Diagnostic = 49,
JSError = 50,
TypeError = 51,
/** TODO this is a DomException type, and should be moved out of here when possible */
DataCloneError = 52,
UrlParse = 20,
Http = 21,
TooLarge = 22,
InvalidSeekMode = 23,
UnixError = 24,
InvalidPath = 25,
ImportPrefixMissing = 26,
Diagnostic = 27,
JSError = 28,
}
// Warning! The values in this enum are duplicated in js/compiler.ts

View file

@ -1,6 +1,6 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
use super::dispatch_json::{Deserialize, JsonOp, Value};
use crate::deno_error::type_error;
use crate::deno_error::other_error;
use crate::fs as deno_fs;
use crate::ops::json_op;
use crate::state::ThreadSafeState;
@ -99,7 +99,7 @@ pub fn op_request_permission(
"env" => Ok(permissions.request_env()),
"plugin" => Ok(permissions.request_plugin()),
"hrtime" => Ok(permissions.request_hrtime()),
n => Err(type_error(format!("No such permission name: {}", n))),
n => Err(other_error(format!("No such permission name: {}", n))),
}?;
Ok(JsonOp::Sync(json!({ "state": perm.to_string() })))
}

View file

@ -1,5 +1,5 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
use crate::deno_error::{permission_denied_msg, type_error};
use crate::deno_error::{other_error, permission_denied_msg};
use crate::flags::DenoFlags;
use ansi_term::Style;
#[cfg(not(test))]
@ -179,8 +179,7 @@ impl DenoPermissions {
}
let url: &str = url.unwrap();
// If url is invalid, then throw a TypeError.
let parsed = Url::parse(url)
.map_err(|_| type_error(format!("Invalid url: {}", url)))?;
let parsed = Url::parse(url).map_err(ErrBox::from)?;
Ok(
self.get_state_net(&format!("{}", parsed.host().unwrap()), parsed.port()),
)
@ -289,7 +288,7 @@ impl DenoPermissions {
"env" => Ok(self.allow_env),
"plugin" => Ok(self.allow_plugin),
"hrtime" => Ok(self.allow_hrtime),
n => Err(type_error(format!("No such permission name: {}", n))),
n => Err(other_error(format!("No such permission name: {}", n))),
}
}
}