windows: mouse sometimes stops working with piped input #440
Comments
|
Hmm.. I don't think it's missing init. Both Not sure what else can break the mouse handling... Can you reproduce the issue? |
|
I can reproduce this. It appears that when it gets in this mode, PeekConsoleInputW is returning 0 even when mouse events occur. I'm not sure what's going on either. I'll try to look into this later today. |
|
What I see is that init_mouse changes the console mode to 0x91 ( ENABLE_PROCESSED_INPUT | ENABLE_MOUSE_INPUT | ENABLE_EXTENDED_FLAGS). Calling GetConsoleMode in win32_kbhit confirms that this value is set. However when EOF is reached, GetConsoleMode suddenly starts returning 0x87. This happens if you press G or just scroll down to the end of the file. I've instrumented every call to SetConsoleMode in less and it is not getting called. I wonder if |
Well, I don't know what Also, I think SetConsoleMode is supposed to be a per-process thing, but not 100% sure. At one stage I thought it might be related to CRLF line endings (regexp.c is checked out with unix line ending on my system, but I thought maybe |
|
I would have expected SetConsoleMode to be per-process, but I don't see how it's getting changed. I've never tried running less under the Visual C debugger, but maybe doing that would help determine if SetConsoleMode is somehow being called from within less. I'm not sure if the debugger allows setting a breakpoint on a system call that's not part of the source. |
|
You could always |
|
Also, if you prefer gdb, then you can try out w64devkit. No installation required, just unzip, double click the main exe (which will open a busybox-w32 shell), then run And quick tip about the shell: it's a fairly complete posix shell, and busybox-w32 has all the important *nix utils (which also have shortcuts at the |
|
For what it's worth, I went back as far as I can build less, which is v540 from 2018, and the issue already exists there. |
|
The problem does not appear to ever happen with a 4096 byte input file, but always happens immediately (no movement required) with a 4097 byte file. 4096 is not the size of any buffer used in less as far as I know (LBUFSIZE is 8192). Instrumenting various places in the code shows that it usually happens in the forw_line_seg function, but at a random place in that function that varies each time it is run. And AFAICT forw_line_seg does not do anything that should cause the console mode to change. This nondeterministic behavior is suggestive that it's happening outside of less. I could work around the problem by resetting the console mode in every call to win32_kbhit or something like that, although that's pretty ugly. @adoxa Jason, pinging you in case you have any ideas. |
|
Some more test cases which fail (executed in busybox sh, cmd /c type regexp.c | cat | LESS= ./less --mouse
cmd /c type regexp.c | printf %s\\n "$(cat)" | LESS= ./less --mouse
cmd /c type regexp.c | (printf %s\\n "$(cat)"; sleep 1) | LESS= ./less --mouse
cmd /c type regexp.c | (x=$(cat); sleep 1; printf %s\\n "$x") | LESS= ./less --mouseAnd two which succeed: cmd /c type regexp.c | (sleep 1; LESS= ./less --mouse)
cmd /c type regexp.c | (x=$(cat); sleep 1; printf %s\\n "$x") | (sleep 1; LESS= ./less --mouse)So it looks as if However, as I said, this also happens with It would be useful to be able to write a |
|
Another approach would be to try to write a minimal Is there anything like strace in Windows that would show what system calls |
|
well, less-like minimal program which uses the mouse would be quite bigger than type-like program :) |
|
I notice the WIN32C version of pclose in less (ttyin.c lines 127-142) manually resets the console mode, and has the comment "Close the pipe, restoring the keyboard (CMD resets it, losing the mouse)". I think Jason wrote that code. This seems to hint at the idea that the console mode get changed when a pipe closes. |
True, but we know exactly what |
IIRC that's BUFSIZ on windows?
Right, and we don't actually care about I'm noticing that with source-highlight it only happens with LESSOPEN. I don't recall if I reproduced it with normal pipe, and I can't now (but it still happens with LESSOPEN). It's also possible that these are two issues (type/SH), even if they have the same symptoms. For reference, I'm attaching my build of source-highlight: source-highlight.rel_3_1_9-8-ge4cf32d+4@avih--win64--2023-09-28.2.zip (just extract it someplace). I'm using it like so (in cmd.exe shell): set "LESSOPEN=| d:/path/to/bin/source-highlight.exe --failsafe --infer-lang -f esc --style-file=esc.style -i %s"
.\less.exe -R --mouse regexp.cInitially the mouse works, but after Can you reproduce it? |
|
Yes, I can reproduce. I also note that it only happens when using a piped LESSOPEN. With a non-piped LESSOPEN there is no issue. I think this is all pointing to a problem where Windows changes the console mode when a program writing into a pipe terminates. |
Right, maybe that would be the place to reset/reinit the console? The source-highlight test case (only happens when reaching [near] the bottom) can definitely be "when the pipe closes". But also, it doesn't happen with |
I think that's because LESSOPEN probably uses So the terminal restoration doesn't happen when it should? As for strace for windows, not that I'm aware of, but I was pointed to these: The first is fairly reputable, no idea about the second. |
|
I tried building v542 (where mouse support was added, with that terminal-reset-on-pclose) and v543, but I can't get the mouse working at all. 64 bit build crashes when using LESSOPEN, 32 bit works, and I also tried without LESSOPEN - I just can't get the mouse wheel scroll to work (when launched as |
|
Well, this (setting the console mode in every call to win32_kbhit) seems to fix it, but I'm not at all convinced that this is the right approach. There may also still be a tiny race, where win32_kbhit sets the console mode and then Windows resets it before PeekConsoleMode is called. diff --git a/screen.c b/screen.c
@@ -1699,8 +1700,8 @@ public void init_mouse(void)
#endif
#endif
@@ -3102,4 +3104,3 @@ public void WIN32textout(char *text, int len)
|
|
When |
Sounds plausible, and annoying.
Agreed, maybe it should do that when piped input reaches EOF? Regardless, two things: You can quote code by placing it between two lines of triple-backticks, the first possibly followed by a syntax-highlight name ( But also, it seems a fairly big diff. Why not something smaller, like so (tested to work): diff --git a/screen.c b/screen.c
index b23c2d7..ddce114 100644
--- a/screen.c
+++ b/screen.c
@@ -3046,10 +3046,15 @@ static int win32_key_event(XINPUT_RECORD *xip)
public int win32_kbhit(void)
{
XINPUT_RECORD xip;
+ DWORD mode;
if (win32_queued_char())
return (TRUE);
+ /* When an input pipe closes (normal or LESSOPEN), the mode may reset */
+ if (mousecap && GetConsoleMode(tty, &mode) && !(mode & ENABLE_MOUSE_INPUT))
+ SetConsoleMode(tty, mode | ENABLE_MOUSE_INPUT);
+
for (;;)
{
DWORD nread;or diff --git a/screen.c b/screen.c
index b23c2d7..6abaaf9 100644
--- a/screen.c
+++ b/screen.c
@@ -3046,10 +3046,22 @@ static int win32_key_event(XINPUT_RECORD *xip)
public int win32_kbhit(void)
{
XINPUT_RECORD xip;
+ DWORD mode;
if (win32_queued_char())
return (TRUE);
+ /* When an input pipe closes (normal or LESSOPEN), the mode may reset */
+ if ((mousecap || vt_enabled) && GetConsoleMode(tty, &mode))
+ {
+ DWORD m = mode
+ | (mousecap ? ENABLE_MOUSE_INPUT : 0)
+ | (vt_enabled ? ENABLE_VIRTUAL_TERMINAL_PROCESSING : 0);
+
+ if (m != mode)
+ SetConsoleMode(tty, m);
+ }
+
for (;;)
{
DWORD nread;As an anecdote, I instrumented it, and with I think that's in line with the observation that with |
|
Yes I know about triple-backtick, but apparently it doesn't work with the < details> tag. I tried both nestings and neither worked. In my change I did do some cleanup that's not strictly necessary to fix the problem. I don't have a strong feeling about it but slightly prefer my change. Regarding doing the mode set only when piped input reaches EOF, that seems a bit trickier and riskier. How would I detect that the input is a pipe, rather than stdin or a fifo (not sure if Windows has fifos)? More to the point, I don't think Window's setting of the mode happens when less READS the EOF; I think it happens when the writer completes, which I don't think can be detected by less. If there's still buffered data in the pipe, less may not read EOF until long after Windows has set the mode. |
IIRC you need an empty line after the opening details line, and/or empty line before the closing detail line. Also, there's a "preview" button to check how it looks before you post, and an "edit" button to modify it after you posted ;)
Right, I thought you might have done this, but it was not easy to read. I don't have a strong opinion either.
You can test I don't think it there are filesystem fifos, but S_IFIFO detects a pipe.
Yup. That's the real issue which I also realized after I suggested it. |
|
Ok, fixed via the discussed kludge in c7ffe20. Windows sucks. |
I don't know if it's right. As far as I can tell, the new However, according to ms docs the bits have different semantics when used with input/output console handles. For instance So I would think that independent input/output modes have to be maintained, and each used with the respective input/output console handle. Also, while we haven't noticed issues with output modes getting reset when the piped command closes, it might be worth "fixing" the output mode as well, together with the input. Not 100% sure about that though, because the input handle was directly connected to where the command was executed, but the output handle is supposedly unrelated to the input pipe command. The right thing (TM) would be to test it, which we haven't done... (that I know of). As for how to "fix" the mode, currently it sets it unconditionally on every $whatever. My hunch is that it would be nicer to set it only if it doesn't match our expectation (what are those? only the set bits? or also the unset ones?). It shouldn't affect a good implementation, but a bad one might reset things unconditionally when the mode is set, even if it's the same as the previous mode. So in retrospect, I think my first diff (if mouse is enabled but the mode doesn't have it, add mouse to the input mode) is the simplest and most correct solution: we only know that the mouse is broken, so if it should be enabled at the mode but isn't - enable it (and possibly move it into the + curr_console_mode = ENABLE_PROCESSED_INPUT | ENABLE_MOUSE_INPUT | ENABLE_EXTENDED_FLAGS; /* disable quick edit */I don't think
And also above this table:
Which, to me, reads as if both And indeed, without But then again, why do we want to disable quick edit at all? It's very useful, and personally It annoys me when it's disabled, because I frequently select-to-copy at the terminal, and when it's disabled one has to hold shift to select text. Also, I didn't test it, but I suspect that, like So I think it's better to keep the default behavior here, without explicitly enabling or disabling quick edit. EDIT: Ignore the extended flags comment. Removing
What doesn't... Unrelated: I noticed that |
|
Hm, yeah there seems to be some confusion of input and output modes. Right now win32_kbhit is setting ENABLE_MOUSE_INPUT on the output handle; I'm surprised this works as it seems like it should be interpreted as ENABLE_LVB_GRID_WORLDWIDE and leave the mouse disabled. I'll review this. As far as the NEWS file, v645 is still "open" (the version number still has an "x" on the end), so things are expected to be in flux. Before an actual v645 release is built I will update the NEWS file (ready_to_release prevents the release build if that isn't done). Ideally I'd update it on the first 645x build; I'll try to do that in the future. |
|
Right, and at DWORD m;
if (GetConsoleMode(tty, &m) && m != curr_console_mode)
SetConsoleMode(tty, curr_console_mode);I tested it, and it eliminates all the unneeded SetConsoleMode calls (which is basically 99.9999% of them). |
|
But why do you want to eliminate SetConsoleMode calls? Is SetConsoleMode slower than GetConsoleMode? |
|
|
But surely something in Windows wouldn't be implemented badly ... oh wait, never mind. :) |
As an anecdote, I tested that on XP(VM)/7/10 and no, it's not slower :) But personally I'd still feel better without setting the console mode 50 times per second during idle. |
|
Ok, take a look at 308cd52. This has several fixes:
It does not yet set the output mode in win32_kbhit. As you say, I'm not sure that's necessary, although I suppose it's unlikely to cause problems if we did so. |
|
Looks much better. Thanks. Two nits: Now if the user doesn't enable quick edit at the console options, then The Get/SetConsoleMode calls do have a return value which indicates success/failure - which is ignored except at the win32_kbhit. I don't think it's likely to fail, but still.. but deciding what to do on failure can be a PITA. Not sure if even a warning is good, except if it's done at most once, and I don't know if it's worth the code. But overall, much better. |
|
The quick edit issue is fixed in 3a4c4c9. I'm not sure about possible ConsoleMode failures. I can't really see anything very useful to do if they fail, except perhaps that if GetConsoleMode fails in get_term, we should set init_console_output_mode to some reasonable default. |
Is it fixed for you? It's not fixed for me as far as I can tell. And looking at the code, while init/deinit mouse look OK, none of them is called when not using curr_console_input_mode = ENABLE_PROCESSED_INPUT | ENABLE_EXTENDED_FLAGS | ENABLE_QUICK_EDIT_MODE;Which enables quick edit... Am I missing something? What say you about the following diff, on top of current master? handle all input mode bits in one placediff --git a/screen.c b/screen.c
index 767cee2..85166ef 100644
--- a/screen.c
+++ b/screen.c
@@ -180,6 +180,8 @@ public int sgr_mode; /* Honor ANSI sequences rather than using above
#if MSDOS_COMPILER==WIN32C
static DWORD init_console_output_mode;
extern DWORD init_console_input_mode;
+extern DWORD base_console_input_mode;
+extern DWORD mouse_console_input_mode;
extern DWORD curr_console_input_mode;
public int vt_enabled = -1; /* Is virtual terminal processing available? */
#endif
@@ -285,10 +287,6 @@ extern int hilite_search;
#if MSDOS_COMPILER==WIN32C
extern int wscroll;
extern HANDLE tty;
-#ifndef ENABLE_EXTENDED_FLAGS
-#define ENABLE_EXTENDED_FLAGS 0x80
-#define ENABLE_QUICK_EDIT_MODE 0x40
-#endif
#else
extern int tty;
#endif
@@ -1699,7 +1697,7 @@ public void init_mouse(void)
ltputs(sc_s_mousecap, sc_height, putchr);
#else
#if MSDOS_COMPILER==WIN32C
- curr_console_input_mode = (curr_console_input_mode | ENABLE_MOUSE_INPUT) & ~(ENABLE_QUICK_EDIT_MODE & init_console_input_mode);
+ curr_console_input_mode = mouse_console_input_mode;
SetConsoleMode(tty, curr_console_input_mode);
#endif
#endif
@@ -1715,7 +1713,7 @@ public void deinit_mouse(void)
ltputs(sc_e_mousecap, sc_height, putchr);
#else
#if MSDOS_COMPILER==WIN32C
- curr_console_input_mode = (curr_console_input_mode & ~ENABLE_MOUSE_INPUT) | (ENABLE_QUICK_EDIT_MODE & init_console_input_mode);
+ curr_console_input_mode = base_console_input_mode;
SetConsoleMode(tty, curr_console_input_mode);
#endif
#endif
diff --git a/ttyin.c b/ttyin.c
index e2e8456..bbd3e66 100644
--- a/ttyin.c
+++ b/ttyin.c
@@ -23,8 +23,20 @@
#define _WIN32_WINNT 0x400
#endif
#include <windows.h>
+
+#ifndef ENABLE_EXTENDED_FLAGS
+#define ENABLE_EXTENDED_FLAGS 0x80
+#define ENABLE_QUICK_EDIT_MODE 0x40
+#endif
+
+#ifndef ENABLE_VIRTUAL_TERMINAL_INPUT
+#define ENABLE_VIRTUAL_TERMINAL_INPUT 0x0200
+#endif
+
public HANDLE tty;
public DWORD init_console_input_mode;
+public DWORD base_console_input_mode;
+public DWORD mouse_console_input_mode;
public DWORD curr_console_input_mode;
#else
public int tty;
@@ -88,9 +100,24 @@ public void open_getchr(void)
tty = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ, &sa,
OPEN_EXISTING, 0L, NULL);
- /* Make sure we get Ctrl+C events. */
+
GetConsoleMode(tty, &init_console_input_mode);
- curr_console_input_mode = ENABLE_PROCESSED_INPUT | ENABLE_EXTENDED_FLAGS | ENABLE_QUICK_EDIT_MODE;
+
+ /*
+ * Make sure we get Ctrl+C events at our modes, without mouse by
+ * default, and without VT input. We don't care about other flags
+ */
+ base_console_input_mode = (init_console_input_mode | ENABLE_PROCESSED_INPUT)
+ & ~(ENABLE_MOUSE_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT);
+
+ /*
+ * enable mouse, and need to disable quick edit, or else it overrides
+ * the mouse (and extended flags is required with disabled quick edit)
+ */
+ mouse_console_input_mode = (base_console_input_mode | ENABLE_MOUSE_INPUT | ENABLE_EXTENDED_FLAGS)
+ & ~ENABLE_QUICK_EDIT_MODE;
+
+ curr_console_input_mode = base_console_input_mode;
SetConsoleMode(tty, curr_console_input_mode);
#else
#if MSDOS_COMPILER
But it is documented that to disable quick edit, one need to set the extended flags, and when the basic mode was without quick edit, adding extended flags indeed disabled quick edit. The fact that quick edit overrides mouse input is not documented explicitly as far as I can tell, but it was mentioned here in 2007 on why it's not on by default:
and also observed elsewhere (good historical overview, which mentions some unfixable gotchas):
Quick edit apparently existed since windows NT 3.1(!), but got enabled by default starting at win10, so existing apps which enable the mouse without disabling quick edit got hit by quick-edit enabled by default and breaking their mouse support... |
|
The QUICK_EDIT bit is not acting like I expect it to. If I uncheck quick edit in the Options tab of the Properties of my cmd window, I confirm that I can't select text with the mouse. Then if I run the program below that just opens a console and closes it, quick edit is now enabled on my console (I can select text and the checkbox is now checked in Properties). I guess this is part of what it means to have quick edit "enabled by default". But it seems really bad that just opening CON changes the mode on the real console. So how can less determine if quick edit is set on the console? If I call GetConsoleMode on this newly-opened handle, it says quick edit is enabled (mode is 0x1f7). As far as I can tell, your proposed patch above has the same problem -- init_console_input_mode is set to 0x1f7 regardless of whether quick edit is enabled in the console. |
That does sound pretty bad.
In my tests I also got once to a state where it changed the terminal checkbox, but then I closed that window and started from scratch in a new window and it was OK. I think this might be part of the unfixable problems mentioned at the second link, because it's stateful, and you can't read the state accurately unless the extended flags bit is set, but if some app (like a previous instance of less) unset the extended bits flags, then it gets into bad territory until you close the window. Or at least that's roughly how I think of it. In my tests with less itself with my last patch above with a new window, if quick edit was disabled, then it remained disabled both with and without mouse enabled in less, and if it was enabled, then it got corectly disabled when using the mouse, and then back to enabled when less exits, and if mouse was not used then it remained enabled the whole time. I can test again with this patch all 4 combos (QE enabled/disabled at the terminal options, with/without |
Almost true, with the exception that I had to start a new window after changing the quick edit checkbox ("Defaults" - the default config, so that a new window will have the updated option). The case where the window starts with quick edit enabled, and using So it's very messy, but I don't think we can have control over it. The bottom line seems to be that to work correctly after changing the QE option manually, a new window is required which starts with the new option value, and then less works correctly with or without Can you confirm this? |
|
I do get better behavior if I only change quick edit in Defaults and then open a new cmd window, rather than editing Properties. But the behavior is still not completely understandable to me; for example:
I've made a correction in 1ce8862. I think this functions as well as can be done, given the weirdnesses of Windows around this feature. |
|
Your Command Prompt properties probably explicitly has QE off - check |
Well, this considers the QE bit while ignoring the extended flags bit, but when the EF is unset, the QE bit doesn't mean much, according to the MS docs and the 2nd link I posted earlier.
Yes, something fishy is going on, and Windows can also have a per-shortcut settings and who knows... But anyway, we can only do what we can do, and that's to modify it as minimally as possible, and eventually restore it as best we can. In our context that means to use the initial value with only the bits we need modified (don't set an absolute value) - like my patch does, and hope for the best... |
|
I don't think c53524e does anything: - curr_console_input_mode = ENABLE_PROCESSED_INPUT | ENABLE_EXTENDED_FLAGS | (ENABLE_QUICK_EDIT_MODE & init_console_input_mode);
+ curr_console_input_mode |= ENABLE_PROCESSED_INPUT | ENABLE_EXTENDED_FLAGS | (ENABLE_QUICK_EDIT_MODE & init_console_input_mode);The initial And even if these bits were added to Case in point, if the user doesn't use
So if QE was enabled but the extended flags and QE bits are unset, then the line above would disable QE. + curr_console_input_mode &= ~(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT);Beyond the fact that this does nothing, because the previous line already ensured these bits are unset, why would you care about those bits anyway? They only affect What you might have cared about in the basic mode, if the previous line was adding bits to The VT input bit is off by default in a new console, but if We also don't care about the initial mouse bit, because So I still think it's best that on startup just ensure that VT input if unset (again, assuming And when enabling the mouse, then the quick edit has to be disabled or else the mouse wont work, so here we'd need to unset quick edit, set extended flags, and set the mouse bit. And if the mouse gets disabled after it was enabled, then get back to the basic mode by simply setting it. it's better than changing bits at Basically, my patch here except that we don't need to disable the mouse explicitly initially. |
|
You're right, I intended to set As far as the ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT bits, I misread the docs to say that they affect ReadConsoleInput rather than ReadConsole, so I think you're right that they don't need to be cleared. The proper handling of the EXTENDED bit is unclear to me. Based solely on the Microsoft docs, it sounds like it would be safe to just leave EXTENDED and QE set to zero on startup, and they will remain unchanged. But it seems safer to explicitly set or clear QE, which according to the docs requires the EXTENDED flag in either case. I don't know how much trust to place in a random blog that contradicts the Microsoft docs. I suspect it's correct but obviously I have no way of testing ancient versions of Windows to confirm it. |
Not sure I get it. Do you mean to explicitly set these bits to zero, even if they had other initial values? I also think this should keep the QE state, but do you think that setting the initial value would change them? I don't think there's evidence or info that setting the original QE and EF bits unmodified would change the QE state? But also, based on that blog post, the docs don't tell the whole story. Here's what it recommends:
So this is what my patch tried to do - touch the extended flags as little as possible, unless we have to (when the mouse is enabled), and then hope it was set initially (which does seems to be the case in my tests), because otherwise all bets are off - but still try to restore by setting the initial value we got, because maybe hopefully something is still able to realize that we want to restore the initial state (the underlaying implementation, based on heuristics or whatever).
I think what it does is to tell a story which the docs omit, but I haven't noticed a contradiction with the docs. Did you? where? For sure, that's only a third party info. But it does seem like some extensive digging was done, and that blog seems generally to the point (that's not the only blog post by the same author), it definitely left good impression on me.
Same. |
Yes. According to the MS doc, you set EXT|QE to enable quick edit, and you set EXT to disable quick edit. That kind of implies that if you don't set either one, quick edit is unchanged, but I wouldn't want to rely on that without clearer documentation. But yeah, I guess leaving both of them unchanged is probably the most conservative thing to do. I'll make another pass, probably tomorrow. |
|
Ok, 06b944e mostly implements your patch, without disabling mouse at startup. |
|
Thanks. Looks good as far as I can tell, but I did not yet put it into real world usage other than some anecdotal tests, as I intend to use it by default with pipe LESSOPEN. But that will happen soon, and I'll report if I notice any issues. (I already know one - it's a bit annoying that there's always a warning to dismiss when pressing |
Well, that didn't happen back then, but I did switch to LESSOPEN by default just now (previously I simulated LESSOPEN-like behavior using a script which piped the filter output into "less" manually). So far I encountered one issue - fixed in #521. I'll keep an eye open for more LESSOPEN related issues,. |
Test case (using
regexp.cfrom thelesssource code):type regexp.c | less --mouseGto go to the end of the document.Expected result:
The document is scrolled backwards (and the prompt/status line stays at the bottom of the screen).
Actual result:
No scroll (or the terminal is scrolled, depending on the terminal).
Mouse buttons (left/right click) also don't work.
Some data points:
type regexp.c | less +Gg --mouse.| type %sor| source-highlight ... %s- the latter is the main reason for filing this issue).less [+Gg] --mouse < regexp.c.busybox cat regexp.c | less +Gg --mouse(using busybox-w32).--mousein less results inIgnore mouse input (press RETURN)and after ENTER the mouse is indeed still not working, but then typing--mouseagain results inUse the mouse for scrolling (press RETURN), and after ENTER the mouse is working again.So it looks like the mouse is de-initialized when using
Gwith piped input (but not from busybox cat, and not with redirected input?), but not re-initialized once theGcommand completes.The text was updated successfully, but these errors were encountered: