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

Document the errors and error codes & causes in the language repo #19

Open
rhamzeh opened this issue Aug 1, 2023 · 1 comment
Open

Comments

@rhamzeh
Copy link
Member

rhamzeh commented Aug 1, 2023

Depends on #99

@ewanharris
Copy link
Member

With the modular models changes we've needed to think about the properties returned as part of the validation errors as they were changed to facilitate mapping of validation errors back to the original source.

In here I'm mostly just using the JS package as an example, it should also apply to Java but It does not apply to Go. I wont include any standard error properties like message, only the properties we specifically declare.

While discussing I'll use the following DSL to demonstrate the structure of the validation errors raised

model
  schema 1.1
type user
type group
  relations
    define member: [user]
    define reader: member and allowed // error here the relation `allowed` does not exist.

Current (main)

Our base interface for an error looks like as follows

Base error
{
	"properties": {
		"msg": string, // the string for the error, such as `the type \`foo\` is a duplicate`
		"line": {
			"start": number, // the starting line number for the error, 1 based not 0 based (in most instances)
			"end": number, // the ending line number for the error, 1 based not 0 based (in most instances)
		},
		"column": {
			"start": number, // the starting column number for the error, 1 based not 0 based (in most instances)
			"end": number, // the ending column number for the error, 1 based not 0 based (in most instances)
		},
	},
	"type": string, // an enum representing the error, this is what the "error code" this issue mentions would document
	"msg": string, // the string for the error, such as `the type \`foo\` is a duplicate`
	"line": {
		"start": number, // the starting line number for the error, 1 based not 0 based (in most instances)
		"end": number, // the ending line number for the error, 1 based not 0 based (in most instances)
	},
	"column": {
		"start": number, // the starting column number for the error, 1 based not 0 based (in most instances)
		"end": number, // the ending column number for the error, 1 based not 0 based (in most instances)
	},
}

For validation errors we then extend this with the following extra data

Metadata
{
	...
	"metadata": {
		"symbol": string, // the "offending" symbol, this is what the line/column data points to. Can be a type or relation name or a more complex string such as `viewer from parent` or `group#member`
		"errorType": string // an enum representing the error, this is what the "error code" this issue mentions would document
	}
}

So the DSL produces an error like below, we can also see that we're duplicating information between the properties property and the top level properties.

Example error
{
  properties: {
    line: { start: 7, end: 7 },
    column: { start: 31, end: 38 },
    msg: 'the relation `allowed` does not exist.'
  },
  type: 'missing-definition',
  line: { start: 7, end: 7 },
  column: { start: 31, end: 38 },
  msg: 'the relation `allowed` does not exist.',
  metadata: { symbol: 'allowed', errorType: 'missing-definition' }
}

Current (modular models branch)

In the modular models branch we needed to extend the information contained within the error for the following reasons:

  • We needed to track the file that an error is associated with to correctly map this in consumers of language, this introduced the file property to the top level of an error
  • We needed to allow the module transformation code map errors back to their original source, in order to achieve this we need to know (depending on the error type) the associated type, relation, or condition. This allows determining the correct line and column information in the original file

So the base interface in the modular models branch now looks as follows

Base error
{
	"properties": {
		"msg": string, // the string for the error, such as `the type \`foo\` is a duplicate`
		"line": {
			"start": number, // the starting line number for the error, 1 based not 0 based (in most instances)
			"end": number, // the ending line number for the error, 1 based not 0 based (in most instances)
		},
		"column": {
			"start": number, // the starting column number for the error, 1 based not 0 based (in most instances)
			"end": number, // the ending column number for the error, 1 based not 0 based (in most instances)
		},
	},
	"type": string, // an enum representing the error, this is what the "error code" this issue mentions would document
	"msg": string, // the string for the error, such as `the type \`foo\` is a duplicate`
	"line": {
		"start": number, // the starting line number for the error, 1 based not 0 based (in most instances)
		"end": number, // the ending line number for the error, 1 based not 0 based (in most instances)
	},
	"column": {
		"start": number, // the starting column number for the error, 1 based not 0 based (in most instances)
		"end": number, // the ending column number for the error, 1 based not 0 based (in most instances)
	},
	"file": string // the filename associated with the error, if there is one
}

For validation errors, the metadata is now expanded to include more data.

Metadata
{
	...
	"metadata": {
		"symbol": string, // the "offending" symbol, this is what the line/column data points to. Can be a type or relation name or a more complex string such as `viewer from parent` or `group#member`
		"errorType": string, // an enum representing the error, this is what the "error code" this issue mentions would document
		"module": string, // if this is part of a modular model, the module that file declared
		"type": string, // the type that the error is associated to (if any)
		"relation": string, // the relation that the error is associated  to (if any)
		"condition": string // the condition that the error is associated  to (if any)
	}
}
Example error

File and module here are undefined as this is not part of a modular model

{
  properties: {
    msg: 'the relation `allowed` does not exist.',
    file: undefined,
    line: { start: 7, end: 7 },
    column: { start: 31, end: 38 }
  },
  type: 'missing-definition',
  line: { start: 7, end: 7 },
  column: { start: 31, end: 38 },
  msg: 'the relation `allowed` does not exist.',
  file: undefined,
  metadata: {
    symbol: 'allowed',
    errorType: 'missing-definition',
    module: undefined,
    relation: 'reader',
    type: 'group'
  }
}

Proposed

First, lets state what it is we need the information in these errors to achieve:

  • Given an error, a consumer should be able to reconstruct most of the msg from the error contained
    • That is to say that with the errorType and supporting information, knowing the template strings should be able to reconstruct the msg, e.g. with missing-definition we need allowed to be in the error metadata, for a more complex error like the `reader` relation definition on type `group` is not valid: `reader` does not exist on `parent`, which is of type `group`. we need to include all information such as the offending symbol reader from parent, the relation and type that contains this symbol, viewer and group respectively, and potentially the referenced relation parent (although this could potentially be parsed by the consumer)
  • Given an error and a DSL representation of a model, a consumer should be able to determine line and column data from the supporting information
    • This is most pertinent to the modular models use case where we construct a single JSON model from multiple files and then validate that and want to map any validation errors back to the original source
  • Given an error, it should be relatively simple for a consumer to access the required information to map this into a representation for their environment
    • That is to say, the main information needed (message, line/column/file data) should be top level properties. Metadata that can be used to provide supporting features (code actions, go to definition) are fine to place in a nested object

So given the above, we could look towards a structure like below. This is realistically just a strawman to push discussion, I think the names are potentially too similar and could cause confusion on what is right to use when, but I'm not 100% comfortable enough with the terminology to suggest better.

Proposed structure
{
	"msg": string, // the string for the error, such as `the type \`foo\` is a duplicate`
	"errorType": string, // an enum representing the error, this is what the "error code" this issue mentions would document
	"line": {
		"start": number, // the starting line number for the error, 1 based not 0 based (in most instances)
		"end": number, // the ending line number for the error, 1 based not 0 based (in most instances)
	},
	"column": {
		"start": number, // the starting column number for the error, 1 based not 0 based (in most instances)
		"end": number, // the ending column number for the error, 1 based not 0 based (in most instances)
	},
	"file": string // the filename associated with the error, if there is one
	"metadata": {
		"symbol": string, // the "offending" symbol, this is what the line/column data points to. Can be a type or relation name or a more complex string such as `viewer from parent` or `group#member`
		"module": string, // the name of the module associated with this error, if any
		"relationName": string, // the name of the relation that contains the symbol, if any
		"typeName": string, // the name of the type that contains the symbol, if any
		"conditionName": string, // the name of the condition that contains the symbol, if any
		"relation": string, // the "offending" relation used, if any
		"type": string, // the "offending" type used, if any
		"condition": string // the "offending" condition used, if any
	}
}
Proposed Error

File and module here are undefined as this is not part of a modular model

{
  errorType: 'missing-definition',
  line: { start: 7, end: 7 },
  column: { start: 31, end: 38 },
  msg: 'the relation `allowed` does not exist.',
  file: undefined,
  metadata: {
    symbol: 'allowed',
    module: undefined,
    relationName: 'reader',
    typeName: 'group',
	relation: 'allowed'
  }
}

To summarise the changes:

  • properties has been dropped, as it only contains data that already exists
  • We maintain all properties that will be required to present an error in an environment at the top level
  • metadata only contains supporting information that can be used to map this error or generate intellisense features
    • For metadata that contains the symbol, we use an xName pattern
    • For metadata that makes up the symbol, we use an x pattern

It isn't immediately clear how we can extend this to support clarifying information such as repo_admin from owner or team#member, given that this information will be parseable from metadata.symbol it might be acceptable to leave this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants