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

Wasm module error behavior #968

Open
janpio opened this issue Nov 25, 2021 · 7 comments
Open

Wasm module error behavior #968

janpio opened this issue Nov 25, 2021 · 7 comments
Labels
domain/schema Issue in the "Schema" domain: Prisma Schema, Introspection, Migrations etc. kind/tech A technical change. topic: wasm
Milestone

Comments

@janpio
Copy link
Contributor

janpio commented Nov 25, 2021

Wasm module probably behaves different than binary, we should figure out how and make sure it is nice for users to understand and create issues based on it.

Related:

@janpio janpio added kind/tech A technical change. topic: wasm labels Nov 25, 2021
@janpio janpio added the domain/schema Issue in the "Schema" domain: Prisma Schema, Introspection, Migrations etc. label Nov 25, 2021
@tomhoule
Copy link
Contributor

2021-11-25_13-11-39
2021-11-25_13-11-28

This is what an unhandled engine prisma-fmt crash would like (a non-helpful JS exception). We have error handlers for most methods though, I'll check what would happen there now.

@tomhoule
Copy link
Contributor

2021-11-25_13-11-50

Here's the output with the panic inside of a try block with a catch calling the error handler. What I understand from this: our error handler swallows errors (it's the same we used for the binary).

@tomhoule
Copy link
Contributor

I double checked and can confirm it's the behaviour I observe.

@tomhoule tomhoule added the process/candidate Candidate for next Milestone. label Nov 25, 2021
@Jolg42
Copy link
Contributor

Jolg42 commented Nov 25, 2021

the panics will throw unhelpful JS exceptions, we'd need to handle them and say what failed and how to report it

@Jolg42 Jolg42 self-assigned this Dec 1, 2021
@Jolg42 Jolg42 removed the process/candidate Candidate for next Milestone. label Dec 1, 2021
@Jolg42 Jolg42 added this to the 3.7.0 milestone Dec 1, 2021
@Jolg42 Jolg42 modified the milestones: 3.7.0, 3.8.0 Dec 21, 2021
@Jolg42
Copy link
Contributor

Jolg42 commented Dec 23, 2021

So the current implementation is only showing error messages for validateTextDocument (which calls lint())
And is surfaced via the onError callback as a notification error.
https://github.com/prisma/language-tools/blob/main/packages/language-server/src/server.ts#L166-L168

Now the question is, do we want to have this notification error for all the methods? 🤔

Jolg42 added a commit that referenced this issue Dec 23, 2021
@Jolg42 Jolg42 modified the milestones: 3.8.0, 3.9.0 Jan 12, 2022
@jkomyno
Copy link
Contributor

jkomyno commented Aug 24, 2022

Note: it may be possible to solve this issue by investigating ideas from this internal thread.

@Jolg42 Jolg42 removed their assignment Dec 9, 2022
@Jolg42
Copy link
Contributor

Jolg42 commented Aug 1, 2023

Note that logging was improved in #1474

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain/schema Issue in the "Schema" domain: Prisma Schema, Introspection, Migrations etc. kind/tech A technical change. topic: wasm
Projects
None yet
Development

No branches or pull requests

4 participants