Catch problems with [tool.poetry.scripts] entries

[christokur]

Pull Request Check List

Resolves: #5885 (partially)

Problem encountered:
Output: too many values to unpack (expected 2) for current project's pyproject.toml i.e. editable install failed
As mentioned in #5885 these failures are cryptic and hard to diagnose. For example, I tried to run poetry via the debugger but then encounter a lot of challenges with subprocess invocations using the debugger options etc.
I eventually tracked down the error in my pyproject.toml to this pattern

[tool.poetry.scripts]
command = "module::main"

Solution:
At a low level I did not wish to substantially change the way the low level code deal with exceptions but I did want to add some context value to the error message the user will see. My proposed change to the _add_script method follows this idea.
I created a test to reproduce the problem, verified that the code changes fixes the problem and added a simple fixture for script related problems like the one I encountered and there are many more test cases we can add there

• Added tests for changed code.
• Updated documentation for changed code.

[radoering]

Suggested change

	msg = f"{exc.args} - Failed to parse script entry point '{script}'"
	raise ValueError(msg) from exc
	raise ValueError(f"Failed to parse script entry point '{script}'") from exc

[christokur]

Several lint tools recommend against interpolation when instantiating Exceptions.
Do you really want this?

[radoering]

Several lint tools recommend against interpolation when instantiating Exceptions.

Sorry, I don't follow. Can you give an example?

My reasoning is that you get the original exception via from exc. Thus, you don't need exc.args it in the derived exception.

[christokur]

Poetry does not display the stack trace so you have to add the exception info to the message so the user can see what the symptom and cause is in one message

[christokur]

Also the example

resources.py:45: [EM102] Exception must not use an f-string literal, assign to variable first

[radoering]

Thanks for the example. I read the reasoning to EM102 and I'm not sure I'm convinced (especially since we are not printing the traceback by default). Normally, we are using string literals and f-strings without assigning it to a variable first, so there is no reason to deviate from that here.

Poetry does not display the stack trace so you have to add the exception info to the message so the user can see what the symptom and cause is in one message

You will see the traceback by running poetry with -vvv. When not in verbose mode, we shouldn't print the original error message since it is not very helpful anyway. If you think that f"Failed to parse script entry point '{script}'" is not enough information, then what about f"Failed to parse script entry point '{script}', ':' not found" or similar?

By the way, we have the same script parsing with the same issue in

poetry/src/poetry/console/commands/run.py

Line 73 in 161b19c

module, callable_ = script.split(":")

[christokur]

This also catches cases where someone does not place any ":"
Are we overthinking this?

[radoering]

Good point, my proposal is not accurate enough. However, I still stand by my initial statement.

IMO ('not enough values to unpack (expected 2, got 1)',) - Failed to parse script entry point '...' is not a good error message. Without any context not enough values to unpack does not really help. Failed to parse script entry point '...' should be good enough. If you want more details you have to run poetry install -v. That way you will get the not enough values to unpack with the line where it happened.

[sneakers-the-rat]

came across #5885 and went to see if anyone had done this PR before submitting it myself, wondering why this was closed? seems like a no brainer. Re the prior discussion, it seems like it would be simple to wrap the whole block of three string splits with try: except: and just say "expected a stript of the form entrypoint = module.submodule:function, got {whatever was input}"

I'm pretty sure the "don't fstring in messages" is more for log messages, where you save time by not interpolating if eg. the message won't be processed due to the loglevel

[radoering]

I assume the author lost interest. It shouldn't be too difficult to push it across the finish line if you address the review feedback.

[sneakers-the-rat]

Cool, done #8898

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.