Repository: electron/electron (language: cpp)

Issue:
ipcRenderer.invoke can't handle custom errors
<!--  As an open source project with a dedicated but small maintainer team, it can sometimes take a long time for issues to be addressed so please be patient and we will get back to you as soon as we can.
-->

### Preflight Checklist
<!-- Please ensure you've completed the following steps by replacing [ ] with [x]-->

* [x] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/master/CONTRIBUTING.md) for this project.
* [x] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/master/CODE_OF_CONDUCT.md) that this project adheres to.
* [x] I have searched the issue tracker for a feature request that matches the one I want to file, without success.

### Problem Description
<!-- Is your feature request related to a problem? Please add a clear and concise description of what the problem is. -->

The product I'm developing now implements a custom error for gRPC to make the error handling. Then, by passing the additional information for the errors made by gRPC to the renderer, I am trying to do a flexible display of error messages with the renderer.

For example, I am trying to use custom errors in the following situations

- display the ID of data that failed to be received
- Handle custom error codes contracted to the gRPC server

However, [ipcRenderer.invoke](https://github.com/electron/electron/blob/659e79fc08c6ffc2f7506dd1358918d97d240147/lib/renderer/api/ipc-renderer.ts#L24-L30) does not allow to receive custom error generated on main process because this function make simple Error() instance with error message.

```ts
ipcRenderer.invoke = async function (channel, ...args) {
  const { error, result } = await ipc.invoke(internal, channel, args);
  if (error) {
    throw new Error(`Error invoking remote method '${channel}': ${error}`);
  }
  return result;
};
```

### Proposed Solution
<!-- Describe the solution you'd like in a clear and concise manner -->

I don't fully understand the [ipc.invoke process described in cpp](https://github.com/electron/electron/blob/72a089262e31054eabd342294ccdc4c414425c99/shell/renderer/api/electron_api_ipc_renderer.cc#L105-L129). If ipc.invoke can return a error thrown by the main process directly, I think the following changes will satisfy my wishes.

```ts
ipcRenderer.invoke = async function (channel, ...args) {
  const { error, result } = await ipc.invoke(internal, channel, args);
  if (error) {
    throw error;
  }
  return result;
};
```

However, the information on the channel drops off. I'd like to come up with a solution here when how to handle custom error policy is decieded.

### Alternatives Considered
<!-- A clear and concise description of any alternative solutions or features you've considered. -->

### Additional Information
<!-- Add any other context about the problem here. -->

If it's the current implementation in relation to the cpp part of the implementation, please let me know. I'm glad to see it. I'm also glad that this custom error policy is being used as an anti If it's a pattern, I'd be happy to point that out as well.


Discussion / hints:
Major issue!

I had to return normal objects inside ipcMain.handle with error information instead of throwing errors since the custom error becomes malformed for the renderer, that means I can not properly do catch handling on renderers invoke Promise.

I pretty much removed all of my .catch handlers in the renderer until this is fixed.
Unfortunately, the proposed change will not fix this issue, as V8's serialization algorithm does not include custom properties on errors. You can test this in Chrome:

```js
window.onmessage = (e) => {
  console.log(e.data.extraProperty)
}
const error = new Error('message')
error.extraProperty = 123
window.postMessage(error)
// => undefined
```

You can see the logic for V8's error serialization here: https://chromium.googlesource.com/v8/v8/+/refs/tags/8.8.44/src/objects/value-serializer.cc#912. It serializes only the type of the error (if it's one of the built-in types), the `message` and `stack` strings, and nothing else.

I'm not sure right now what the best path forward is, but here are a few of my thoughts:

1. I think we should actually maybe be exposing _less_ information here, rather than more. Error messages can contain sensitive data in some situations, and the Web sanitizes error messages when they cross between contexts: https://html.spec.whatwg.org/multipage/webappapis.html#muted-errors. We should perhaps do the same; the main process should be considered "cross-origin" from any renderer context.
2. If we _do_ want to expose more error information, we should probably not expose the stack trace from the main process, as that stack won't make sense in the renderer.
3. Exposing extra properties could be done by serializing a property object alongside the error, for example (in `web-contents.ts`):

```js
    event._throw = (error: Error) => {
      console.error(`Error occurred in handler for '${channel}':`, error);
      event.sendReply({ error: { name: error.name, message: error.message, extra: { ...error } } });
    };
```

and then deserialized on the renderer side as:

```js
  const { error, result } = await ipc.invoke(internal, channel, args);
  if (error) {
    const errorObj = new Error(`Error invoking remote method '${channel}': ${error.name} ${error.message}`)
    Object.assign(errorObj, error.extra)
    throw errorObj;
  }
```

These techniques could easily be applied by an app to wrap `ipcMain.handle` and `ipcRenderer.invoke` with their custom error-handling logic. For example:


```js
// Main process
const encodeError = (e) => {
  return {name: e.name, message: e.message, extra: {...e}}
}

const handleWithCustomErrors = (channel, handler) => {
  ipcMain.handle(channel, async (...args) => {
    try {
      return {result: await Promise.resolve(handler(...args))}
    } catch (e) {
      return {error: encodeError(e)}
    }
  })
}

// Renderer process
const decodeError = ({name, message, extra}) => {
  const e = new Error(message)
  e.name = name
  Object.assign(e, extra)
  return e
}

const invokeWithCustomErrors = async (...args) => {
  const {error, result} = await ipcRenderer.invoke(...args)
  if (error) {
    throw decodeError(error)
  }
  return result
}
```
My two cents here.

I agree with the issue report. This problem is considerably important from my perspective.

I am using a workaround similar to the one proposed by @nornagon. However, it is quite confusing for another member of the team that this works like this and it should be explained that when the promise is rejected, the result is different to what is normally expected, and certain information such as the original message is lost (now it is encoded in the other message), the type of the error (constructor name), as well as any additional attributes.

I think the ipcRenderer and ipcMain implementations should encapsulate the solution and document the behavior. The solution can be something like @nornagon or any other mechanism (not necessarily that the error that is obtained is exactly the same that was emitted from the main process) to get the proper error without resorting to workarounds that may need a lot of code duplication or may be prompted by people unfamiliar with the issues here for errors.
```
> However, it is quite confusing for another member of the team that this works like this and it should be explained that when the promise is rejected, the result is different to what is normally expected, and certain information such as the original message is lost (now it is encoded in the other message), the type of the error (constructor name), as well as any additional attributes.

> I think the ipcRenderer and ipcMain implementations should encapsulate the solution and document the behavior.

Indeed, it's quite confusing to beginners at least, that you receive a Promise but its error differs from the original error. So it's not like a real/pure Promise. If the feature is designed to return Promise, then I believe it should act like one, so it's less misleading. Or it could be designed to return a callback with a ResultOrSanitizedError-like response-wrapper object. I don't experience this problem though since from day one use the IPC-based communication only and realize that serialization is taking place (I use the [serialize-error](https://github.com/sindresorhus/serialize-error) module in [this type-safe IPC](https://github.com/vladimiry/electron-rpc-api) wrapper).
I'm not sure I understand the confusion around the Promise behavior; there's nothing particularly unusual about a promise rejecting with an error. It doesn't seem surprising to me that sending a message to another process and receiving an error in response would produce a not-identical error to the one thrown in the remote process. The error message that _is_ received specifically explains what has happened: it says "Error invoking remote method '${channel}'".

Perhaps it would be less confusing if the error message thrown in the renderer actually didn't include any details about the remote error? It would, for example, be less surprising for developers to discover that none of the extra properties of the error they threw were present if the error was obviously a reconstructed, different error.

Note that it is ultimately impossible to create a perfect copy of the thrown error in the calling process, because it's possible for user code to create and throw custom objects as errors, and there is no way for us to recreate such custom objects in a totally separate process. (e.g. `class MyError extends Error { /* ... */ }`). V8 itself handles its builtin error types (`RangeError`, `TypeError` and so on) but any support for other error types must necessarily be handled by custom user code.
@nornagon What is confusing is that what causes the error (the method invoked in the other process) is not what the client gets from the invocation. It is "handled" by an intermediary, discarded, and then another error with completely different semantics is what the client gets from the invocation.

This is not even documented and is completely misleading. What anyone would expect with the promise mechanism, and even with exceptions, is that it propagates the error information until it is handled properly. Instead, what I understand from the IPC mechanism (and it is not documented) is that the contract, in general, is that the invocation should never fail, and the error should be communicated by a different mechanism, so it really looks like you have to have two totally different mechanisms to handle errors at this point: 1) one for the IPC communication (which doesn't really make much sense to me. What's the use of this?), 2) and a different one to really communicate any of the possible failure causes of the process that is invoked.

I'm not saying that it should be the exact same Error, but this should be very well-documented, and also it will very helpful if the error sent to the client, has the information on the original Error in a easy, elegant, and natural way to obtain. I mean, I don't see as a good solution to have to parse the message string to get the original message. Also, it's a problem when you have custom errors with additional attributes like a status code. If it's well documented, it could be as simple as communication a custom IpcError with an additional attribute with the original error thrown as a JSON string or something like that as long the decision is well-documented in the API.
I spent 4 hours trying to figure out this issue, finally running into this thread and @nornagon's workaround which solved it for me. This should at the very least be documented in ipcMain docpage. I think the workaround should probably be included as well until a time we get a PR to fix this, if possible.
I believe this should be tagged as a bug, not an enhancement.  You lose access to the object you try to pass back, so promise rejection is broken.
Just spent more than 2 hours to create a solid workaround for this issue. It's MOST DEFINITELY NOT how it should work. This is a bug.
@m4heshd by solid workaround do you mean something different and/or safer than what @nornagon came up with? can you share if you have something new?
@ash0x0 It's just something very specific to my use case but still hell dealing with this.
Just an FYI, I tried @nornagon's approach using `ipcRender` with `contextBridge` and it won't work. I still see the serialization issue once the error is thrown again after the error has been decoded.
In case you have context isolation enabled (officially recommended) and need to pass custom error types / properties to the UI component it gets even uglier because errors will be stripped of any custom attributes in the preload script.

refer to https://github.com/electron/electron/issues/25596

To use [nornagon](https://github.com/nornagon) example for the renderer you would need to write some kind of middleware function to 'parse' the error outside of the preload script in the renderer script but its really ugly, needs to be super well documented.

```
export async function callElectronApi(
  fn: (...args: any[]) => any,
  ...args: any[]
): Promise<any> {
  const { result, error } = await fn(...args)
  if (error) {
    throw decodeError(error)
  }
  return result
}
```

And then use like

`callElectronApi(window.electronApi.myFunction, arg1)`

please lmk if you find a better solution


Fix patch:
diff --git a/docs/api/ipc-renderer.md b/docs/api/ipc-renderer.md
index dae89527aa42c..d17b761286883 100644
--- a/docs/api/ipc-renderer.md
+++ b/docs/api/ipc-renderer.md
@@ -96,14 +96,6 @@ Algorithm][SCA], just like [`window.postMessage`][], so prototype chains will no
 included. Sending Functions, Promises, Symbols, WeakMaps, or WeakSets will
 throw an exception.
 
-> **NOTE:** Sending non-standard JavaScript types such as DOM objects or
-> special Electron objects will throw an exception.
->
-> Since the main process does not have support for DOM objects such as
-> `ImageBitmap`, `File`, `DOMMatrix` and so on, such objects cannot be sent over
-> Electron's IPC to the main process, as the main process would have no way to decode
-> them. Attempting to send such objects over IPC will result in an error.
-
 The main process should listen for `channel` with
 [`ipcMain.handle()`](./ipc-main.md#ipcmainhandlechannel-listener).
 
@@ -126,6 +118,21 @@ If you need to transfer a [`MessagePort`][] to the main process, use [`ipcRender
 
 If you do not need a response to the message, consider using [`ipcRenderer.send`](#ipcrenderersendchannel-args).
 
+> **Note**
+> Sending non-standard JavaScript types such as DOM objects or
+> special Electron objects will throw an exception.
+>
+> Since the main process does not have support for DOM objects such as
+> `ImageBitmap`, `File`, `DOMMatrix` and so on, such objects cannot be sent over
+> Electron's IPC to the main process, as the main process would have no way to decode
+> them. Attempting to send such objects over IPC will result in an error.
+
+> **Note**
+> If the handler in the main process throws an error,
+> the promise returned by `invoke` will reject.
+> However, the `Error` object in the renderer process
+> will not be the same as the one thrown in the main process.
+
 ### `ipcRenderer.sendSync(channel, ...args)`
 
 * `channel` string


Test patch: