Prettify Error message and stack display, fix #32 . #40

mc-zone · 2017-05-30T16:44:30Z

1. Add path and operation name into error message, make it easier to figure out.

before (error.message):

no such file or directory
illegal operation on a directory
operation not permitted
file already exists
not a directory

after:

ENOENT: no such file or directory, unlinkSync '/test/abcd'
EISDIR: illegal operation on a directory, writeFileSync '/test/dir'
EPERM: operation not permitted, rmdirSync '/'
EEXIST: file already exists, mkdirSync '/test/dir'
ENOTDIR: not a directory, mkdirSync '/test/file'

2. Change first line of stack to use correct message, instance of [object Object].

before (beginning of error.stack):

Error: [object Object]
    at MemoryFileSystem.readFileSync (...

after:

MemoryFileSystemError: ENOENT: no such file or directory, readFileSync '/fail/file'
    at MemoryFileSystem.readFileSync ( ...

It will fix #32 .

1. Add path and operation name into error message, make it easier to figure out. 2. Change first line of stack to use correct message, instance of [object Object].

codecov · 2017-05-30T16:56:26Z

Codecov Report

Merging #40 into master will increase coverage by 0.11%.
The diff coverage is 97.56%.

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   96.12%   96.24%   +0.11%     
==========================================
  Files           3        4       +1     
  Lines         284      293       +9     
  Branches       65       67       +2     
==========================================
+ Hits          273      282       +9     
  Misses         11       11

Impacted Files	Coverage Δ
lib/join.js	`100% <ø> (ø)`	⬆️
lib/MemoryFileSystemError.js	`100% <100%> (ø)`
lib/MemoryFileSystem.js	`99.06% <100%> (-0.03%)`	⬇️
lib/normalize.js	`83.33% <83.33%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b96090...5fcc26c. Read the comment docs.

…ore call `captureStackTrace` (found in node V6.x). Use strict mode. Prettify tests.

jsf-clabot · 2017-05-31T13:12:11Z

All committers have signed the CLA.

mc-zone · 2017-06-01T02:25:05Z

Coverage decreased (-0.07%) to 96.403%

I've already add the tests. I actually don't know whether I should, and how to cover this 0.07...
But I think it's satisfying for review now. :p @sokra @SpaceK33z @TheLarkInn

sokra · 2017-06-01T09:31:14Z

lib/MemoryFileSystemError.js

+		if(operation && path) {
+			this.message += `, ${operation} \'${path}\'`;
+		} else if(path) {
+			this.message += `, \'${path}\'`;


This line is not covered. If unused remove it.

sokra · 2017-06-01T09:33:17Z

lib/MemoryFileSystemError.js

+			Error.captureStackTrace(this, this.constructor);
+		}
+	}
+	findOperation() {


Don't do that.

Instead pass the operation name in the error constructor.

mc-zone · 2017-06-01T11:28:39Z

Done, review again please. @sokra

sokra · 2017-06-01T13:18:16Z

lib/MemoryFileSystem.js

 		current[path[i]] = {"":true};
 		return;
 	}

 	_remove(_path, name, testFn) {
 		const path = pathToArray(_path);
 		if(path.length === 0) {
-			throw new MemoryFileSystemError(errors.code.EPERM, _path);
+			throw new MemoryFileSystemError(errors.code.EPERM, _path, "remove");


remove is not an operation: pass rmdir or unlink

Fixed. my bad.

sokra · 2017-06-02T08:46:37Z

.eslintrc.json

@@ -202,7 +202,6 @@
            "error",
            "always"
        ],
-        "strict": "error",


I guess this doesn't have to be removed. You have a "use strict" in the file?

Yeap, I extracted MemoryFileSystemError to a standalone file. So add "use strict" for it's classes declaration, otherwise it will broken in node v4.

@sokra Does this have any side effects?

mc-zone · 2017-06-07T09:13:54Z

Excuse me, @sokra maybe too busy to notice here? 😄
Must I remove the "use strict"? Could you give any reason? Please.

mc-zone · 2017-06-14T08:08:46Z

uh, so, the codacy can't pass if not removed it...

sokra · 2017-06-14T11:57:55Z

hmmm... weird

sokra · 2017-06-18T17:49:15Z

Thanks

mc-zone added 2 commits May 31, 2017 00:28

Prettify Error message and stack display, fix webpack#32 .

228b515

1. Add path and operation name into error message, make it easier to figure out. 2. Change first line of stack to use correct message, instance of [object Object].

fix codacy hint

833cf1d

mc-zone closed this May 31, 2017

mc-zone reopened this May 31, 2017

Fix stack message missing caused by visit and invoke stack getter bef…

59849c5

…ore call `captureStackTrace` (found in node V6.x). Use strict mode. Prettify tests.

mc-zone closed this May 31, 2017

mc-zone reopened this May 31, 2017

mc-zone force-pushed the master branch from e7981cf to db65e09 Compare May 31, 2017 13:13

try to fix codacy hint

9f015b5

mc-zone force-pushed the master branch from db65e09 to 9f015b5 Compare June 1, 2017 02:07

mc-zone mentioned this pull request Jun 1, 2017

Fix: ENOENT will display erroneous path in message #35

Closed

sokra suggested changes Jun 1, 2017

View reviewed changes

mc-zone added 3 commits June 1, 2017 19:14

remove stack search

0fa2435

resolve conflict

e9e4cd4

fix codacy hint

501f381

sokra suggested changes Jun 1, 2017

View reviewed changes

fix operation on _remove

1c36bc6

sokra reviewed Jun 2, 2017

View reviewed changes

revert removing of strict check

4a19442

linting fixes

5fcc26c

TheLarkInn approved these changes Jun 14, 2017

View reviewed changes

sokra approved these changes Jun 18, 2017

View reviewed changes

sokra merged commit 67cec91 into webpack:master Jun 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prettify Error message and stack display, fix #32 . #40

Prettify Error message and stack display, fix #32 . #40

mc-zone commented May 30, 2017 •

edited

Loading

codecov bot commented May 30, 2017 •

edited

Loading

jsf-clabot commented May 31, 2017 •

edited

Loading

mc-zone commented Jun 1, 2017 •

edited

Loading

sokra Jun 1, 2017

sokra Jun 1, 2017

mc-zone Jun 1, 2017

mc-zone commented Jun 1, 2017

sokra Jun 1, 2017

mc-zone Jun 1, 2017

sokra Jun 2, 2017

mc-zone Jun 2, 2017 •

edited

Loading

mc-zone Jun 4, 2017

mc-zone commented Jun 7, 2017

mc-zone commented Jun 14, 2017

sokra commented Jun 14, 2017

sokra commented Jun 18, 2017

Prettify Error message and stack display, fix #32 . #40

Prettify Error message and stack display, fix #32 . #40

Conversation

mc-zone commented May 30, 2017 • edited Loading

1. Add path and operation name into error message, make it easier to figure out.

2. Change first line of stack to use correct message, instance of [object Object].

codecov bot commented May 30, 2017 • edited Loading

Codecov Report

jsf-clabot commented May 31, 2017 • edited Loading

mc-zone commented Jun 1, 2017 • edited Loading

sokra Jun 1, 2017

Choose a reason for hiding this comment

sokra Jun 1, 2017

Choose a reason for hiding this comment

mc-zone Jun 1, 2017

Choose a reason for hiding this comment

mc-zone commented Jun 1, 2017

sokra Jun 1, 2017

Choose a reason for hiding this comment

mc-zone Jun 1, 2017

Choose a reason for hiding this comment

sokra Jun 2, 2017

Choose a reason for hiding this comment

mc-zone Jun 2, 2017 • edited Loading

Choose a reason for hiding this comment

mc-zone Jun 4, 2017

Choose a reason for hiding this comment

mc-zone commented Jun 7, 2017

mc-zone commented Jun 14, 2017

sokra commented Jun 14, 2017

sokra commented Jun 18, 2017

mc-zone commented May 30, 2017 •

edited

Loading

codecov bot commented May 30, 2017 •

edited

Loading

jsf-clabot commented May 31, 2017 •

edited

Loading

mc-zone commented Jun 1, 2017 •

edited

Loading

mc-zone Jun 2, 2017 •

edited

Loading