feat(script-compiler): add error responses

[otariidae]

ref: #72

• add TextlintWorkerCommandResponseLintError and TextlintWorkerCommandResponseFixError
• add error handling for Textlint worker

[azu]

textlint's lintText/fixText does not reject any result in normally.
It will be unexpected result.

lint:error will make misleading.
textlint return lint error as messages instead of exception.

I think lint:error is redundant.
It is just error(unexpected error).

[otariidae]

OK. So, all your reviews considered, you mean we need single error response type to handle unexpected errors, right?
Fixed in 1bc8669.

[azu]

Is it possible to make this just a global error?
Ideally, it would be more convenient if users could get it as a default error event in WebWorker.

https://developer.mozilla.org/en-US/docs/Web/API/Worker/error_event

// user can catch the error as global error
worker.addEventListner("error", event => {
  console.error(event);
  console.error(event.error);
});

Current error handling looks like complex.

If make this just a global error, user can just listen "error" event for error handling.

// AbortSignals can be used for event listner
// https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#add_an_abortable_listener
const controller = new AbortController();
const onMessage = (event) => {
  // do somthing
  ontroller.abort(); // unlisten
}
// error handling
const onError = (event) => {
  // do error handling
  controller.abort(); // unlisten
}
worker.addEventListner("message", onMessage, { signal: controller.signal });
worker.addEventListner("error", onError, { signal: controller.signal });

📝 Custom Error may be needed.

class TextlintError extends Error {
  constructor(message, id){
   // https://javascript.info/custom-errors
  }
}

[otariidae]

Looks great for me! But ErrorEvent.event seems to be always null.
Moreover I found addEventListener("error", ...) can handle thrown Error but cannot handle promise rejection. I am considering another idea based on your review.

[otariidae]

I found error event listener can handle promise rejection in the worker if the worker handle unhandledrejection event and use reportError.

// in the worker
self.addEventListener("unhandledrejection", error => {
    reportError(error)
})

But I haven't found how to handle id through error event listener.

[azu]

Probably, reportError allow to report ErrorEvent which include custom Error object.

const error = new Error('description');
error.id = "test-1";
self.reportError(new ErrorEvent('textlint error', {
    error : error,
    message : 'description',
}));

• reportError() global function - Web APIs | MDN
  • HTML Standard

Technically, it could be written as follows.

class LintError extends Error { }
lintText(...).catch(error => {
  reportError(new ErrorEvent('textlint error', {
    error : new LintError("description", { id }),
    message : 'description',
}))

However, Safari/Node.js/Deno does not support reportError in WebWorker yet.
https://developer.mozilla.org/en-US/docs/Web/API/reportError

[otariidae]

Here is a wip commit f2fcc24 including debug comments to share the situation.
In this case I got a ErrorEvent with Uncaught #<EventError> message and null error, so reportError with ErrorEvent wrapping TextlintError does not work for me.

[azu]

Fmm, It may be worker limitation.

• https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2

We can not pass the details of the error in worker via error event.

[otariidae]

Thank you for your information.
Another option for less complex error handling I think is that all errors in the worker are handled through worker messages instead of throwing Error or calling reportError. It seems almost practically safe to omit handling of error and messaageerror. I finally understand why other libraries such as comlink have their own error message protocol.
If we want to promise more safety, worker can post error messages when the worker gets error or unhandledrejection on the worker side:

self.addEventListener('error', (error) => {
    // handling all unexpected error on the worker side and notify the client through message
    self.postMessage({
        command: 'error',
        error,
    })
})

[azu]

another option for less complex error handling I think is that all errors in the worker are handled through worker messages instead of throwing Error or calling reportError

Yes. it is good that keep it simple.

[otariidae]

implemented in 1375633
Textlint worker handles all possible errors and post error messages. It simplified client-side error handling by omitting error and messageerror event

[azu]

Probably, reportError allow to report ErrorEvent which include custom Error object.

const error = new Error('description');
error.id = "test-1";
self.reportError(new ErrorEvent('textlint error', {
    error : error,
    message : 'description',
}));

• reportError() global function - Web APIs | MDN
  • HTML Standard

Technically, it could be written as follows.

class LintError extends Error { }
lintText(...).catch(error => {
  reportError(new ErrorEvent('textlint error', {
    error : new LintError("description", { id }),
    message : 'description',
}))

However, Safari/Node.js/Deno does not support reportError in WebWorker yet.
https://developer.mozilla.org/en-US/docs/Web/API/reportError

[azu]

Errors should be handled by the caller.

update().catch(error => ...) or create some wrapper for logging.

[otariidae]

implemented in e181e81
But I am not so much familiar with layered error handling technique so please review if it may be wrong for you.

[azu]

Promise.prototype.finally return a Promise.
So you can return the promise.

Suggested change

	fixPromise.finally(() => {
	controller.abort();
	});
	return fixPromise;
	return fixPromise.finally(() => {
	controller.abort();
	});;

[otariidae]

implemented in 7ea7580
also removed Promise variable definition as it looks redundant too in this case

[azu]

same here.

Although in most cases there is no difference.

[azu]

Almost LGTM.
Thanks for work!

[azu]

Thank you for your patience!
https://github.com/textlint/editor/releases/tag/v0.15.0 🎉

[otariidae]

Thank you!
This PR was harder than I expected but is a very good experience for me.