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

First-minute UX: user is flooded with errors if they forget about adding @SerializabilityTrait #123

Open
PawelLipski opened this issue Oct 29, 2021 · 5 comments
Labels
nice to have Not very strictly needed for ASH to be usable usability Relates to reduction of boilerplate etc.

Comments

@PawelLipski
Copy link
Collaborator

That's what I've got while testing on Laguna project... I've activated ash plugin on the main module, and got like 100 errors of ......Response is used as Akka message but does not extend a trait annotated with org.virtuslab.ash.annotation.SerializabilityTrait. Passing an object of class NOT extending SerializabilityTrait as a message may cause Akka to fall back to Java serialization during runtime..

This is technically correct... but in fact I'd rather expect an error like there is no class annotated with @SerializabilityTrait on this module's classpath or sth like that, b/c that's the actual error to be addressed first.

@PawelLipski PawelLipski added the usability Relates to reduction of boilerplate etc. label Oct 29, 2021
@PawelLipski PawelLipski changed the title First-minute UX: user is flooded is errors if they forget about adding @SerializabilityTrait First-minute UX: user is flooded with errors if they forget about adding @SerializabilityTrait Nov 10, 2021
@LukaszKontowski LukaszKontowski self-assigned this Jun 28, 2022
@LukaszKontowski
Copy link
Contributor

LukaszKontowski commented Jun 30, 2022

This is technically correct... but in fact I'd rather expect an error like there is no class annotated with @SerializabilityTrait on this module's classpath or sth like that, b/c that's the actual error to be addressed first.

In my opinion the only possibility to achieve this would be to search all files in client's source code directory before checking the first compilation unit by the SerializabilityCheckerCompilerPluginComponent. As each apply on this component's phase knows only one compilation unit (from one currently compiled .scala file) - we cannot check all the module for annotations from just current compilation unit.

This check on files is of course possible, but it would affect performance negatively. And, in my opinion, if someone starts to use ASH - he/she would at least take a look at our README which describes that @SerializabilityTrait annotation is the basis for our plugin usage.

@PawelLipski Do we want to try it or just leave it as it is?

@PawelLipski
Copy link
Collaborator Author

search all files in client's source code directory

Huh... so basically add an extra phase of compilation, dedicated to just finding whether there's any @SerializabilityTrait anywhere, right? 🤔 or sth even more hackish (like accessing source files directly "on the disk"?)

@LukaszKontowski
Copy link
Contributor

LukaszKontowski commented Jun 30, 2022

Huh... so basically add an extra phase of compilation, dedicated to just finding whether there's any @SerializabilityTrait anywhere, right? 🤔 or sth even more hackish (like accessing source files directly "on the disk"?)

Yes, that's right - and if it should be another phase of compilation - it would start before the serializability checker of course

@PawelLipski
Copy link
Collaborator Author

Hmmmm sounds like quite a lot of engineering... and a performance overhead for a very short-lived benefit (only really matters in the developer's first ever contact with this tool). Not closing the issue, but marking as nice to have and unassigning you then

@PawelLipski PawelLipski added the nice to have Not very strictly needed for ASH to be usable label Jun 30, 2022
@LukaszKontowski
Copy link
Contributor

Note: maybe this could be done by using the https://www.scala-lang.org/api/2.13.0/scala-compiler/scala/tools/nsc/Global.html#findMemberFromRoot(fullName:Global.this.Name):Global.this.Symbol - to be checked and tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nice to have Not very strictly needed for ASH to be usable usability Relates to reduction of boilerplate etc.
Projects
None yet
Development

No branches or pull requests

3 participants