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

Internal error: Supposedly valid service did not have command #803

Open
leumasme opened this issue Jun 30, 2023 · 0 comments
Open

Internal error: Supposedly valid service did not have command #803

leumasme opened this issue Jun 30, 2023 · 0 comments

Comments

@leumasme
Copy link

leumasme commented Jun 30, 2023

A config like

"foo": {
  "service": true,
  "dependencies": [
    "bar"
  ]
},

is not highlighted as invalid by the extension.
Having such a config seems to cause the extension to crash repeatedly

Error: Internal error: Supposedly valid service did not have command
    at on._checkForCyclesAndSortDependencies (c:\Users\Temm\.vscode\extensions\google.wireit-0.6.0\server.js:42:20959)
    at on.analyzeFiles (c:\Users\Temm\.vscode\extensions\google.wireit-0.6.0\server.js:42:4600)
    at async ii.getDiagnostics (c:\Users\Temm\.vscode\extensions\google.wireit-0.6.0\server.js:42:24688)
    at async Au (c:\Users\Temm\.vscode\extensions\google.wireit-0.6.0\server.js:42:29636)
[Error - 2:30:09 AM] The wireit server server crashed 5 times in the last 3 minutes. The server will not be restarted. See the output for more information.

It seems that the extension does not properly check the rule that service scripts must have a command

wireit/src/analyzer.ts

Lines 1529 to 1537 in 28cb48b

if (config.service !== undefined) {
// We should already have created an invalid script at this point, so we
// should never get here. We throw here to convince TypeScript that this
// is guaranteed.
if (config.command === undefined) {
throw new Error(
'Internal error: Supposedly valid service did not have command'
);
}

we should never get here. We throw here to convince TypeScript that this is guaranteed.

Very amusing comment.

Notably, a service script with an invalid dependency will properly throw

"foo": {
  "service": true,
  "dependencies": [
    "doesnotexist"
  ]
},

results in these two errors:

A "service" script must have a "command". wireit
Cannot find script named "bar" in package "s:\Dev\projectname" wireit
rictic added a commit that referenced this issue Sep 4, 2023
This way later scripts can assume locally-valid means no failures in the script that are locally determinable.

I think this was the original intent, and some code seems to assume this. See e.g. #803
rictic added a commit that referenced this issue Sep 4, 2023
* Don't upgrade a script to locally-valid if it has errors

This way later scripts can assume locally-valid means no failures in the script that are locally determinable.

I think this was the original intent, and some code seems to assume this. See e.g. #803

* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant