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

Error constructor syntax is vulnerable to typos #393

Closed
hasithaa opened this issue Jan 15, 2020 · 8 comments
Closed

Error constructor syntax is vulnerable to typos #393

hasithaa opened this issue Jan 15, 2020 · 8 comments
Assignees
Labels
Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks incompatible Resolving issue may not be backwards compatible status/pending Design is agreed and waiting to be added

Comments

@hasithaa
Copy link
Contributor

I made a typo, I typed massage, instead of the message.

error("SERVICE ERROR", massage = "internal error occurred", cause = result);

One solution is to use string-literal (similar to mapping constructor) for non-field names defined in detail's record type descriptor. But it will be an incompatible change.

@hasithaa hasithaa added Area/Lang Relates to the Ballerina language specification design/dislike Do not like something about the design labels Jan 15, 2020
@jclark
Copy link
Collaborator

jclark commented Jan 16, 2020

This is related to #268.

@jclark jclark added design/usability Design does not work well for some tasks and removed design/dislike Do not like something about the design labels Jan 16, 2020
@jclark jclark self-assigned this Jan 16, 2020
@jclark jclark added the incompatible Resolving issue may not be backwards compatible label Jan 16, 2020
@jclark jclark added this to the 2020R1 milestone Jan 16, 2020
@hasithaa
Copy link
Contributor Author

The old syntax #67 didn't have this problem, because it uses the record literal syntax.

error("ioerror", {filename: s});

@rdhananjaya
Copy link
Member

@hasithaa Maybe I'm missing something somewhere, but I can't see how old syntax solves this issue. In my view, the root cause for the typo is message field of error detail record being optional and error detail record being open, which facilitated the new field massage. Even with the record literal syntax this issue is still possible right.

@jclark
Copy link
Collaborator

jclark commented Feb 3, 2020

#268 fixed the problem for mapping constructors.

@jclark jclark modified the milestones: 2020R1, 2020R2 Feb 24, 2020
@jclark
Copy link
Collaborator

jclark commented Apr 8, 2020

There will be a similar problem with functional constructors for mapping values as in #2.

@jclark jclark modified the milestones: 2020R3, 2020R2 Apr 24, 2020
@jclark
Copy link
Collaborator

jclark commented Apr 24, 2020

Here's one possible approach.

  • message and cause move from the detail record into the error itself
    • the type is string? and error? respectively
    • they both default to () if not specified
    • they are specified by positional arguments to the error constructor
    • lang.error provides message and cause functions to access them
  • the required type of detail record is map<anydata|readonly> (with no predefined fields)
  • if the error constructor uses a type that does not specify a detail record type with individual fields, then named arguments can be used to specify arbitrary fields
  • if the error constructor uses a type that does specify a detail record type with individual fields, then the named arguments can only be used to specify those fields; other fields must be specified with ... and a mapping constructor (not sure if this functionality is really necessary)

If you want to specify a cause but not a message, you can specify () for the first argument (which is the same as not specifying it).

This doesn't address how to generate a message if the user doesn't explicitly specify one. There's lots of potential complexity there: i18n, formatting the message to include the fields from the detail record. Another point is that this is tied more to the distinct type than the detail record.

@jclark jclark added the status/pending Design is agreed and waiting to be added label Apr 28, 2020
@jclark
Copy link
Collaborator

jclark commented May 1, 2020

This is solved by the revised error design in #509.

@jclark jclark closed this as completed May 1, 2020
@jclark
Copy link
Collaborator

jclark commented Jun 18, 2020

This will be handled now in #550.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks incompatible Resolving issue may not be backwards compatible status/pending Design is agreed and waiting to be added
Projects
None yet
Development

No branches or pull requests

3 participants